Re: [PATCH] Add option to grub-probe to accept system devices as arguments

Re: [PATCH] Add option to grub-probe to accept system devices as arguments

[Fabian Greffrath]

[-- Attachment #1: Type: text/plain, Size: 1474 bytes --]

> You put that function in a separate file, which indicates it is meant to be
> a general-purpose function, but its spec is constrained by the code in
> grub-probe (i.e. you strdup() not because it is needed for the purpose of
> this function, but because grub-probe already calls free()).

That's right. Consequently there are two alternatives left:

- grub_util_check_block_device() returns the pointer instead of strdup() and the
call to free() in grub-probe is conditional upon if argument_is_device is true
or false; which I consider an ugly hack.

- I keep getroot.{c,h} untouched and include these four lines in grub-probe;
which is a pity because I consider the function quite usefull, though still not
perfect (see above).

In the context of the second alternative, the attached (draft!) patch leads to 
the following results:

	$ mount
	/dev/hda1 on / type ext3 (rw,errors=remount-ro)
	/dev/hda6 on /home type ext3 (rw)
	$ su
	
	# grub-probe
	No path or device is specified.
	Try ``grub-probe --help'' for more information.
	# grub-probe /home
	ext2
	# grub-probe /homer
	grub-probe: error: cannot stat /homer.
	
	# grub-probe /dev/hda1
	ext2
	# grub-probe --target=drive
	No path or device is specified.
	Try ``grub-probe --help'' for more information.
	# grub-probe --target=drive /home
	(hd0,6)
	# grub-probe --target=drive /homer
	grub-probe: error: cannot stat /homer.
	
	# grub-probe --target=drive /dev/hda1
	(hd0,1)

How do you like that?

Cheers,
Fabian


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: grub-probe-20080212-1.diff --]
[-- Type: text/x-patch; name="grub-probe-20080212-1.diff", Size: 3609 bytes --]

diff -Nurp grub2-1.96+20080210/util/grub-probe.c grub2-1.96+20080210.new/util/grub-probe.c
--- grub2-1.96+20080210/util/grub-probe.c	2008-02-09 11:49:29.000000000 +0100
+++ grub2-1.96+20080210.new/util/grub-probe.c	2008-02-12 10:16:45.000000000 +0100
@@ -1,4 +1,4 @@
-/* grub-probe.c - probe device information for a given path */
+/* grub-probe.c - probe device information for a given path or device */
 /*
  *  GRUB  --  GRand Unified Bootloader
  *  Copyright (C) 2005,2006,2007,2008 Free Software Foundation, Inc.
@@ -100,7 +100,7 @@ probe_partmap (grub_disk_t disk)
 }
 
 static void
-probe (const char *path)
+probe (const char *argument)
 {
   char *device_name;
   char *drive_name = NULL;
@@ -108,10 +108,18 @@ probe (const char *path)
   char *filebuf_via_grub = NULL, *filebuf_via_sys = NULL;
   int abstraction_type;
   grub_device_t dev = NULL;
-  
-  device_name = grub_guess_root_device (path);
+  struct stat st;
+
+  if (stat (argument, &st) < 0)
+     grub_util_error ("cannot stat %s.\n", argument);
+
+  if (S_ISBLK (st.st_mode))
+    device_name = strdup (argument);
+  else
+    device_name = grub_guess_root_device (argument);
+
   if (! device_name)
-    grub_util_error ("cannot find a device for %s.\n", path);
+    grub_util_error ("cannot find a device for %s.\n", argument);
 
   if (print == PRINT_DEVICE)
     {
@@ -178,21 +186,18 @@ probe (const char *path)
 
   if (print == PRINT_FS)
     {
-      struct stat st;
       grub_fs_t fs;
 
-      stat (path, &st);
-
       if (st.st_mode == S_IFREG)
 	{
 	  /* Regular file.  Verify that we can read it properly.  */
 
 	  grub_file_t file;
-	  grub_util_info ("reading %s via OS facilities", path);
-	  filebuf_via_sys = grub_util_read_image (path);
+	  grub_util_info ("reading %s via OS facilities", argument);
+	  filebuf_via_sys = grub_util_read_image (argument);
 	  
-	  grub_util_info ("reading %s via GRUB facilities", path);
-	  asprintf (&grub_path, "(%s)%s", drive_name, path);
+	  grub_util_info ("reading %s via GRUB facilities", argument);
+	  asprintf (&grub_path, "(%s)%s", drive_name, argument);
 	  file = grub_file_open (grub_path);
 	  filebuf_via_grub = xmalloc (file->size);
 	  grub_file_read (file, filebuf_via_grub, file->size);
@@ -242,9 +247,9 @@ usage (int status)
 	     "Try ``grub-probe --help'' for more information.\n");
   else
     printf ("\
-Usage: grub-probe [OPTION]... PATH\n\
+Usage: grub-probe [OPTION]... [PATH|DEVICE]\n\
 \n\
-Probe device information for a given path.\n\
+Probe device information for a given path or device.\n\
 \n\
   -m, --device-map=FILE     use FILE as the device map [default=%s]\n\
   -t, --target=(fs|drive|device|partmap|abstraction)\n\
@@ -264,7 +269,7 @@ int
 main (int argc, char *argv[])
 {
   char *dev_map = 0;
-  char *path;
+  char *argument;
   
   progname = "grub-probe";
   
@@ -321,10 +326,10 @@ main (int argc, char *argv[])
   if (verbosity > 1)
     grub_env_set ("debug", "all");
 
-  /* Obtain PATH.  */
+  /* Obtain PATH or DEVICE.  */
   if (optind >= argc)
     {
-      fprintf (stderr, "No path is specified.\n");
+      fprintf (stderr, "No path or device is specified.\n");
       usage (1);
     }
 
@@ -334,7 +339,7 @@ main (int argc, char *argv[])
       usage (1);
     }
 
-  path = argv[optind];
+  argument = argv[optind];
   
   /* Initialize the emulated biosdisk driver.  */
   grub_util_biosdisk_init (dev_map ? : DEFAULT_DEVICE_MAP);
@@ -343,7 +348,7 @@ main (int argc, char *argv[])
   grub_init_all ();
 
   /* Do it.  */
-  probe (path);
+  probe (argument);
   
   /* Free resources.  */
   grub_fini_all ();

Re: [PATCH] Add option to grub-probe to accept system devices as arguments

[Robert Millan]

On Tue, Feb 12, 2008 at 10:49:37AM +0100, Fabian Greffrath wrote:
> >You put that function in a separate file, which indicates it is meant to be
> >a general-purpose function, but its spec is constrained by the code in
> >grub-probe (i.e. you strdup() not because it is needed for the purpose of
> >this function, but because grub-probe already calls free()).
> 
> That's right. Consequently there are two alternatives left:
> 
> - grub_util_check_block_device() returns the pointer instead of strdup() 
> and the
> call to free() in grub-probe is conditional upon if argument_is_device is 
> true
> or false; which I consider an ugly hack.
> 
> - I keep getroot.{c,h} untouched and include these four lines in grub-probe;
> which is a pity because I consider the function quite usefull, though still 
> not
> perfect (see above).

That's why I proposed to pass both path and device, and let either of them be
NULL.  When you do this:

  - You can determine whether `device' is provided (hence you can skip
    converting from `path') simply with "if (device)".

  - You can make the free() part independant of any other indications by
    issuing:

      if (device)
        free (device);
      if (path)
        free (path);

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: [PATCH] Add option to grub-probe to accept system devices as arguments

[Fabian Greffrath]

Please have a look at the patch I provided in my last mail, too. This is a 
different approach which requires no further command line option but *detects* 
whether the given argument is a path or a device.

Re: [PATCH] Add option to grub-probe to accept system devices as arguments

[Fabian Greffrath]

Fabian Greffrath schrieb:
> Please have a look at the patch I provided in my last mail, too. This is 
> a different approach which requires no further command line option but 
> *detects* whether the given argument is a path or a device.

Ping! ;-)

Re: [PATCH] Add option to grub-probe to accept system devices as arguments

[Robert Millan]

On Tue, Feb 12, 2008 at 02:17:11PM +0100, Fabian Greffrath wrote:
> Please have a look at the patch I provided in my last mail, too. This is a 
> different approach which requires no further command line option but 
> *detects* whether the given argument is a path or a device.

I find that too unreliable.  How do you reproduce current behaviour with
the command "grub-probe -t fs /dev/sda1" ?

I'd prefer exposed user options than hidden behaviour like this one.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: [PATCH] Add option to grub-probe to accept system devices as arguments

[Fabian Greffrath]

Robert Millan schrieb:
> I find that too unreliable.  How do you reproduce current behaviour with
> the command "grub-probe -t fs /dev/sda1" ?

I don't see the problem. Why should this be unreliable?

If the device does not exist, grub-probe will throw a "cannot stat /dev/sda1" 
error with and without the patch. If the device does exist, grub-probe will 
throw a "cannot find device for /dev/sda1" error without the patch and will 
return "ext2" with the patch applied.

See the following examples (I have /dev/hda1, but not /dev/sda1):

	Patched grub-probe:
	# grub-probe -t fs /dev/sda1
	grub-probe: error: cannot stat /dev/sda1.
	# grub-probe -t fs /dev/hda1
	ext2
	
	Current grub-probe:
	# grub-probe -t fs /dev/sda1
	grub-probe: error: Cannot stat `/dev/sda1'
	# grub-probe -t fs /dev/hda1
	grub-probe: error: cannot find a device for /dev/hda1.

> I'd prefer exposed user options than hidden behaviour like this one.

I still don't see the problem. The program will behave absolutely predictable: 
If the argument is a block device it will print the device's file system. If the 
argument is a valid path, it will prints the corresponding device's file system. 
If the argument is none of both, it will throw an error.

Cheers,
Fabian

Re: [PATCH] Add option to grub-probe to accept system devices as arguments

[Robert Millan]

On Wed, Feb 13, 2008 at 05:51:53PM +0100, Fabian Greffrath wrote:
> Robert Millan schrieb:
> >I find that too unreliable.  How do you reproduce current behaviour with
> >the command "grub-probe -t fs /dev/sda1" ?
> 
> I don't see the problem. Why should this be unreliable?
> 
> If the device does not exist, grub-probe will throw a "cannot stat 
> /dev/sda1" error with and without the patch. If the device does exist, 
> grub-probe will throw a "cannot find device for /dev/sda1" error without 
> the patch and will return "ext2" with the patch applied.
> 
> See the following examples (I have /dev/hda1, but not /dev/sda1):
> 
> 	Patched grub-probe:
> 	# grub-probe -t fs /dev/sda1
> 	grub-probe: error: cannot stat /dev/sda1.
> 	# grub-probe -t fs /dev/hda1
> 	ext2
> 	
> 	Current grub-probe:
> 	# grub-probe -t fs /dev/sda1
> 	grub-probe: error: Cannot stat `/dev/sda1'
> 	# grub-probe -t fs /dev/hda1
> 	grub-probe: error: cannot find a device for /dev/hda1.

The last one sounds like a bug.

> >I'd prefer exposed user options than hidden behaviour like this one.
> 
> I still don't see the problem. The program will behave absolutely 
> predictable: If the argument is a block device it will print the device's 
> file system. If the argument is a valid path, it will prints the 
> corresponding device's file system. If the argument is none of both, it 
> will throw an error.

Yes, but does the user know that?  Which interface is easier to figure out
for the user, one in which different actions require different parameters,
or one where the same parameters act in completely different way depending
on the file?

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: [PATCH] Add option to grub-probe to accept system devices as arguments

[Fabian Greffrath]

Robert Millan schrieb:
> The last one sounds like a bug.

Yes, it's from the current *unpatched* grub-probe.

> Yes, but does the user know that? 

We could tell the user via 'grub-probe --help' that he may pass either a path or 
a block device as an argument.

> Which interface is easier to figure out
> for the user, one in which different actions require different parameters,
> or one where the same parameters act in completely different way depending
> on the file?

There are no different actions! Whether you give a valid path or a block device 
as an argument, the program behaves exactly the same. It does even behave 
exactly as before (i.e. unpatched) if given a path. Accepting devices as 
argument is simply an *added* feature.

Re: [PATCH] Add option to grub-probe to accept system devices as arguments

[Robert Millan]

On Thu, Feb 14, 2008 at 09:33:48AM +0100, Fabian Greffrath wrote:
> Robert Millan schrieb:
> >The last one sounds like a bug.
> 
> Yes, it's from the current *unpatched* grub-probe.

I mean that it should find the disk that contains /dev/hda1 device _node_.

> >Which interface is easier to figure out
> >for the user, one in which different actions require different parameters,
> >or one where the same parameters act in completely different way depending
> >on the file?
> 
> There are no different actions! Whether you give a valid path or a block 
> device as an argument, the program behaves exactly the same. It does even 
> behave exactly as before (i.e. unpatched) if given a path. Accepting 
> devices as argument is simply an *added* feature.

Consider the following:

$ cat > /boot/grub/device.map << EOF
(hd0)	/dev/device1
(hd1)	/dev/device2
EOF
$ mount /dev/device1 /mnt
$ touch /mnt/file
$ grub-probe -t drive /mnt/file
???
$ ln -s nowhere /mnt/symlink
$ grub-probe -t drive /mnt/symlink
???
$ mkfifo /mnt/fifo
$ grub-probe -t drive /mnt/fifo
???
$ mknod /mnt/device2
$ grub-probe -t drive /mnt/device2
???

Is the same output supposed to be printed in all "???" ?  If not, what is in
each one?

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: [PATCH] Add option to grub-probe to accept system devices as arguments

[Fabian Greffrath]

Robert Millan schrieb:
> I mean that it should find the disk that contains /dev/hda1 device _node_.

Maybe, but I do not believe this has anything to do with my patch.

> Is the same output supposed to be printed in all "???" ?  If not, what is in
> each one?

In all of these cases the 'if (S_ISBLK (st.st_mode))' test introduced by my 
patch will fail and thus grub-probe's behaviour will be exactly as before.

Re: [PATCH] Add option to grub-probe to accept system devices as arguments

[Fabian Greffrath]

Now that the FSF has received my copyright assignment paper back, I'd like to 
become productive on this issue again. From what we've discussed so far I assume 
that you prefer
- the introduction of a new command line option
- passing two parameters to probe(), a path and a device, of which one is NULL
- not exporting the IF_BLKDEV test into a new function as long as strdup() of 
the returned string cannot be avoided.

Cheers,
Fabian

Re: [PATCH] Add option to grub-probe to accept system devices as arguments

[Fabian Greffrath]

Hi Robert,

> You need to list every function or variable separately.  [...]
> If you use the -p option to diff it'll be easier to check it just by reading the
> patch.

OK, thanks, I didn't know this. Next time... ;)

> Is there really a need to strdup() it?

Yep. If you return the pointer (i.e. to `path') you'll get memory corruption as 
soon as you try to free (device_name).

> I find it confusing that you change the meaning of the `path' variable without
> renaming it.  Also, the variable that describes what `path' is
> (argument_is_device) is passed separately.

Allright. Maybe the `path' variable should be renamed to `argument'.

> What would you think of a scheme where both are passed as strings and either
> can be NULL? 

Well, I prefer the idea to have the argument stored in one string and have a 
separate boolean switch that tells if the string contains a device name or a 
path. But I am always open to suggestions. ;)

> I suspect advertising it here might lead users to think they can pass a device
> to it, without caring about [...] this option.

Allright, this should be changed to something like this:
"Probe device information for a given path (or device, if the -d option is given)."

Thanks!

Cheers,
Fabian

Re: [PATCH] Add option to grub-probe to accept system devices as arguments

[Robert Millan]

On Mon, Feb 11, 2008 at 04:48:26PM +0100, Fabian Greffrath wrote:
> >Is there really a need to strdup() it?
> 
> Yep. If you return the pointer (i.e. to `path') you'll get memory 
> corruption as soon as you try to free (device_name).

You put that function in a separate file, which indicates it is meant to be
a general-purpose function, but its spec is constrained by the code in
grub-probe (i.e. you strdup() not because it is needed for the purpose of
this function, but because grub-probe already calls free()).

In case of a general-purpose function, it doesn't make much sense to strdup
it.  Consider what would a caller have to do in case it wasn't using free()
already:

ret = check (device);
if (! ret)
  fatal ("%s is bad", device);
free (ret);

instead of just:

if (! check (device))
  fatal ("%s is bad", device);

> >I find it confusing that you change the meaning of the `path' variable 
> >without
> >renaming it.  Also, the variable that describes what `path' is
> >(argument_is_device) is passed separately.
> 
> Allright. Maybe the `path' variable should be renamed to `argument'.

That works too ... but please put them together (i.e. both as arguments to
probe()).

> >I suspect advertising it here might lead users to think they can pass a 
> >device
> >to it, without caring about [...] this option.
> 
> Allright, this should be changed to something like this:
> "Probe device information for a given path (or device, if the -d option is 
> given)."

Ok.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

[PATCH] Add option to grub-probe to accept system devices as arguments

[Fabian Greffrath]

[-- Attachment #1: Type: text/plain, Size: 297 bytes --]

The following patch adds to grub-probe the ability to accept system devices as
arguments and e.g. convert between system devices and GRUB drives.

This patch is improved over the one from my previous posting and has some minor
issues fixed. Please use this version instead. It is all my work.





[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: grub-probe.diff --]
[-- Type: text/x-patch; name="grub-probe.diff", Size: 3859 bytes --]

2008-02-11  Fabian Greffrath  <fabian.greffrath@web.de>

	* util/grub-probe.c: Add new parameter '-d, --device'. If this is set,
	grub-probe expects the given argument to be a block device. All of the
	'--target' parameters work with this option as well. If the '--device'
	parameter is not set, grub-probe will work as before.

	* util/getroot.c (grub_util_check_block_device): New public function
	that returns the given argument if it is a block device and returns
	NULL else.

	* util/getroot.h (grub_util_check_block_device): Export function.

diff -Naru grub2-1.96+20080203~/include/grub/util/getroot.h grub2-1.96+20080203/include/grub/util/getroot.h
--- grub2-1.96+20080203~/include/grub/util/getroot.h	2008-01-12 16:11:56.000000000 +0100
+++ grub2-1.96+20080203/include/grub/util/getroot.h	2008-02-08 12:49:52.000000000 +0100
@@ -29,5 +29,6 @@
 char *grub_get_prefix (const char *dir);
 int grub_util_get_dev_abstraction (const char *os_dev);
 char *grub_util_get_grub_dev (const char *os_dev);
+char *grub_util_check_block_device (const char *blk_dev);
 
 #endif /* ! GRUB_UTIL_GETROOT_HEADER */
diff -Naru grub2-1.96+20080203~/util/getroot.c grub2-1.96+20080203/util/getroot.c
--- grub2-1.96+20080203~/util/getroot.c	2008-01-12 16:11:56.000000000 +0100
+++ grub2-1.96+20080203/util/getroot.c	2008-02-08 12:49:52.000000000 +0100
@@ -327,3 +327,17 @@
 
   return grub_dev;
 }
+
+char *
+grub_util_check_block_device (const char *blk_dev)
+{
+  struct stat st;
+
+  if (stat (blk_dev, &st) < 0)
+    grub_util_error ("Cannot stat `%s'", blk_dev);
+
+  if (S_ISBLK (st.st_mode))
+    return strdup (blk_dev);
+  else
+    return 0;
+}
diff -Naru grub2-1.96+20080203~/util/grub-probe.c grub2-1.96+20080203/util/grub-probe.c
--- grub2-1.96+20080203~/util/grub-probe.c	2008-01-25 23:33:57.000000000 +0100
+++ grub2-1.96+20080203/util/grub-probe.c	2008-02-08 12:50:13.000000000 +0100
@@ -50,6 +50,7 @@
 };
 
 int print = PRINT_FS;
+static unsigned int argument_is_device = 0;
 
 void
 grub_putchar (int c)
@@ -84,9 +85,18 @@
   int abstraction_type;
   grub_device_t dev = NULL;
   
-  device_name = grub_guess_root_device (path);
+  if (argument_is_device)
+    device_name = grub_util_check_block_device (path);
+  else
+    device_name = grub_guess_root_device (path);
+
   if (! device_name)
-    grub_util_error ("cannot find a device for %s.\n", path);
+    {
+      if (argument_is_device)
+        grub_util_error ("%s is not a block device.\n", path);
+      else
+        grub_util_error ("cannot find a device for %s.\n", path);
+    }
 
   if (print == PRINT_DEVICE)
     {
@@ -201,6 +211,7 @@
 
 static struct option options[] =
   {
+    {"device", no_argument, 0, 'd'},
     {"device-map", required_argument, 0, 'm'},
     {"target", required_argument, 0, 't'},
     {"help", no_argument, 0, 'h'},
@@ -217,10 +228,11 @@
 	     "Try ``grub-probe --help'' for more information.\n");
   else
     printf ("\
-Usage: grub-probe [OPTION]... PATH\n\
+Usage: grub-probe [OPTION]... [PATH|DEVICE]\n\
 \n\
-Probe device information for a given path.\n\
+Probe device information for a given path or device.\n\
 \n\
+  -d, --device              given argument is a system device, not a path\n\
   -m, --device-map=FILE     use FILE as the device map [default=%s]\n\
   -t, --target=(fs|drive|device|partmap|abstraction)\n\
                             print filesystem module, GRUB drive, system device, partition map module or abstraction module [default=fs]\n\
@@ -246,13 +258,17 @@
   /* Check for options.  */
   while (1)
     {
-      int c = getopt_long (argc, argv, "m:t:hVv", options, 0);
+      int c = getopt_long (argc, argv, "dm:t:hVv", options, 0);
       
       if (c == -1)
 	break;
       else
 	switch (c)
 	  {
+	  case 'd':
+	    argument_is_device = 1;
+	    break;
+
 	  case 'm':
 	    if (dev_map)
 	      free (dev_map);

Re: [PATCH] Add option to grub-probe to accept system devices as arguments

[Robert Millan]

On Mon, Feb 11, 2008 at 09:53:15AM +0100, Fabian Greffrath wrote:
> The following patch adds to grub-probe the ability to accept system devices 
> as
> arguments and e.g. convert between system devices and GRUB drives.
> 
> This patch is improved over the one from my previous posting and has some 
> minor
> issues fixed. Please use this version instead. It is all my work.

Hi Fabian,

Some comments on this one.

> ???2008-02-11  Fabian Greffrath  <fabian.greffrath@web.de>
> 
> 	* util/grub-probe.c: Add new parameter '-d, --device'. If this is set,
> 	grub-probe expects the given argument to be a block device. All of the
> 	'--target' parameters work with this option as well. If the '--device'
> 	parameter is not set, grub-probe will work as before.

You need to list every function or variable separately.  E.g:

	* util/grub-probe.c (argument_is_device): New variable.
	(probe): Blah blah.
	(main): Etc.

If you use the -p option to diff it'll be easier to check it just by reading the
patch.

> +char *
> +grub_util_check_block_device (const char *blk_dev)
> +{
> +  struct stat st;
> +
> +  if (stat (blk_dev, &st) < 0)
> +    grub_util_error ("Cannot stat `%s'", blk_dev);
> +
> +  if (S_ISBLK (st.st_mode))
> +    return strdup (blk_dev);
> +  else
> +    return 0;
> +}

Is there really a need to strdup() it?

> diff -Naru grub2-1.96+20080203~/util/grub-probe.c grub2-1.96+20080203/util/grub-probe.c
> --- grub2-1.96+20080203~/util/grub-probe.c	2008-01-25 23:33:57.000000000 +0100
> +++ grub2-1.96+20080203/util/grub-probe.c	2008-02-08 12:50:13.000000000 +0100
> @@ -50,6 +50,7 @@
>  };
>  
>  int print = PRINT_FS;
> +static unsigned int argument_is_device = 0;
>  
>  void
>  grub_putchar (int c)
> @@ -84,9 +85,18 @@
>    int abstraction_type;
>    grub_device_t dev = NULL;
>    
> -  device_name = grub_guess_root_device (path);
> +  if (argument_is_device)
> +    device_name = grub_util_check_block_device (path);
> +  else
> +    device_name = grub_guess_root_device (path);

I find it confusing that you change the meaning of the `path' variable without
renaming it.  Also, the variable that describes what `path' is
(argument_is_device) is passed separately.

What would you think of a scheme where both are passed as strings and either
can be NULL?  E.g.

probe (char *path, char *device_name)
{
  if (path == NULL)
    {
      if (grub_util_check_block_device (device_name) == -1)
        return -1;
    }
  else
    device_name = grub_guess_root_device (path);
}

> -Usage: grub-probe [OPTION]... PATH\n\
> +Usage: grub-probe [OPTION]... [PATH|DEVICE]\n\
>  \n\
> -Probe device information for a given path.\n\
> +Probe device information for a given path or device.\n\

I suspect advertising it here might lead users to think they can pass a device
to it, without caring about ...

> +  -d, --device              given argument is a system device, not a path\n\

... this option.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)