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

[Andrey Borzenkov <arvidjaar@gmail.com>]

What about this patch? Looks correct to me and reflects actual behavior.


В Sun, 02 Jun 2013 16:48:23 +0200
Francesco Lavra <francescolavra.fl@gmail.com> пишет:

> 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  <francescolavra.fl@gmail.com>
> 
> 	* 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  <francescolavra.fl@gmail.com>
> 
> 	* 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
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel