Re: [PATCH 2/2] ls core command: handle listing of the root directory

[Andrey Borzenkov]

В Thu, 28 Nov 2013 07:31:54 +0100
"Vladimir 'phcoder' Serbinenko" <address@hidden> пишет:

> this very confusing with 2 opposite patches in same mail. I don't have my
> keys here. Feel free to commit documentation patch. If you commit on
> Francesco's behalf don't forget --author

Done.

> On Nov 27, 2013 7:42 PM, "Andrey Borzenkov" <address@hidden> wrote:
> 
> > What about this patch? Looks correct to me and reflects actual behavior.
> >
> >
> > В Sun, 02 Jun 2013 16:48:23 +0200
> > Francesco Lavra <address@hidden> пишет:
> >
> > > On 05/18/2013 12:26 PM, Francesco Lavra wrote:
> > > > On 05/12/2013 12:56 PM, Francesco Lavra wrote:
> > > >> Currently, listing of the root directory of a device with the command:
> > > >> ls (device_name)
> > > >> requires the underlying filesystem driver to handle an empty path
> > > >> string as if it was the root directory path "/". This introduces
> > > >> duplicated code across the different filesystem drivers. If a given
> > > >> filesystem driver does not implement special handling of the empty
> > > >> path string, the above command gives "error: invalid file name `'."
> > > >> This error happens for instance with the ext4 filesystem.
> > > >> The best place to handle correctly the empty path string and transform
> > > >> it in "/" is the function grub_core_cmd_ls(), so that handling from
> > > >> each filesystem driver is not required anymore.
> > > >
> > > > After revision 5010, issuing the ls command with a device name as
> > > > parameter gives a response such as:
> > > > (device_name): Filesystem is <filesystem name>.
> > > > But grub.texi says:
> > > > "
> > > > @deffn Command ls [arg @dots{}]
> > > > List devices or files.
> > > >
> > > > With no arguments, print all devices known to GRUB.
> > > >
> > > > If the argument is a device name enclosed in parentheses (@pxref{Device
> > > > syntax}), then list all files at the root directory of that device.
> > > >
> > > > If the argument is a directory given as an absolute file name
> > (@pxref{File
> > > > name syntax}), then list the contents of that directory.
> > > > @end deffn
> > > > "
> > > >
> > > > Which is the correct behavior?
> > >
> > > Depending on the answer to the above question, you might want to
> > > consider applying one of the two patches below, as you see fit.
> > >
> > > Regards,
> > > Francesco
> > >
> > > 2013-06-02  Francesco Lavra  <address@hidden>
> > >
> > >       * docs/grub.texi (ls): Fix command description in case of a device
> > name
> > >       passed as argument.
> > >
> > > === modified file 'docs/grub.texi'
> > > --- docs/grub.texi    2013-05-11 05:23:26 +0000
> > > +++ docs/grub.texi    2013-06-02 14:35:24 +0000
> > > @@ -4049,7 +4049,7 @@
> > >  With no arguments, print all devices known to GRUB.
> > >
> > >  If the argument is a device name enclosed in parentheses (@pxref{Device
> > > -syntax}), then list all files at the root directory of that device.
> > > +syntax}), then print the name of the filesystem of that device.
> > >
> > >  If the argument is a directory given as an absolute file name
> > (@pxref{File
> > >  name syntax}), then list the contents of that directory.
> > >
> > >
> > > 2013-06-02  Francesco Lavra  <address@hidden>
> > >
> > >       * grub-core/kern/corecmd.c (grub_core_cmd_ls): Fix listing of root
> > >       directory contents.
> > >
> > > === modified file 'grub-core/kern/corecmd.c'
> > > --- grub-core/kern/corecmd.c  2013-06-02 14:23:14 +0000
> > > +++ grub-core/kern/corecmd.c  2013-06-02 14:28:24 +0000
> > > @@ -147,13 +147,11 @@
> > >
> > >        if (! *path)
> > >       {
> > > -       if (grub_errno == GRUB_ERR_UNKNOWN_FS)
> > > -         grub_errno = GRUB_ERR_NONE;
> > > -
> > > -       grub_printf ("(%s): Filesystem is %s.\n",
> > > -                    device_name, fs ? fs->name : "unknown");
> > > +       /* The argument is a device name: list all files at the root
> > directory
> > > +          of the device. */
> > > +       path = (char *) "/";
> > >       }
> > > -      else if (fs)
> > > +      if (fs)
> > >       {
> > >         (fs->dir) (dev, path, grub_mini_print_files, NULL);
> > >         grub_xputs ("\n");
> > >
> > > _______________________________________________
> > > Grub-devel mailing list
> > > address@hidden
> > > https://lists.gnu.org/mailman/listinfo/grub-devel
> >
> >
> > _______________________________________________
> > Grub-devel mailing list
> > address@hidden
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> >