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

[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");