[PATCH] grub-probe -t prefix, -t all

[PATCH] grub-probe -t prefix, -t all

[Christian Franke]

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

This patch adds '-t prefix', '-t all', and allows multiple -t options to 
be specified.

'-t prefix' was initially added to test grub_get_prefix() on Cygwin, but 
may be useful in scripts.

Christian

2007-10-29  Christian Franke  <franke@computer.org>

	* util/grub-probe.c (#define PRINT_*): Change to bitmasks
	to allow multiple '-t type' options.
	Add PRINT_PREFIX (-t prefix)
	(probe): Likewise.
	Types now passed as parameter.
	(main): Likewise.
	Add '-t all'



[-- Attachment #2: grub2-grub-probe-prefix.patch --]
[-- Type: text/x-patch, Size: 4143 bytes --]

--- grub2.orig/util/grub-probe.c	2007-07-22 21:17:26.000000000 +0200
+++ grub2/util/grub-probe.c	2007-10-14 17:28:29.000000000 +0200
@@ -39,12 +39,11 @@
 #define _GNU_SOURCE	1
 #include <getopt.h>
 
-#define PRINT_FS	0
-#define PRINT_DRIVE	1
-#define PRINT_DEVICE	2
-#define PRINT_PARTMAP	3
-
-int print = PRINT_FS;
+#define PRINT_FS	0x01
+#define PRINT_DRIVE	0x02
+#define PRINT_PREFIX	0x04
+#define PRINT_DEVICE	0x08
+#define PRINT_PARTMAP	0x10
 
 void
 grub_putchar (int c)
@@ -70,7 +69,7 @@
 }
 
 static void
-probe (const char *path)
+probe (const char *path, int types)
 {
   char *device_name;
   char *drive_name = NULL;
@@ -81,20 +80,36 @@
   if (! device_name)
     grub_util_error ("cannot find a device for %s.\n", path);
 
-  if (print == PRINT_DEVICE)
+  if (types & PRINT_DEVICE)
     {
       printf ("%s\n", device_name);
-      goto end;
+      types &= ~PRINT_DEVICE;
+      if (types == 0)
+        goto end;
     }
 
   drive_name = grub_util_get_grub_dev (device_name);
   if (! drive_name)
     grub_util_error ("cannot find a GRUB drive for %s.\n", device_name);
   
-  if (print == PRINT_DRIVE)
+  if (types & PRINT_DRIVE)
     {
       printf ("(%s)\n", drive_name);
-      goto end;
+      types &= ~PRINT_DRIVE;
+      if (types == 0)
+        goto end;
+    }
+
+  if (types & PRINT_PREFIX)
+    {
+      char * prefix = grub_get_prefix (path);
+      if (! prefix)
+        grub_util_error ("cannot find prefix for %s.\n", path);
+      printf ("%s\n", prefix);
+      free (prefix);
+      types &= ~PRINT_PREFIX;
+      if (types == 0)
+        goto end;
     }
 
   grub_util_info ("opening %s", drive_name);
@@ -102,7 +117,7 @@
   if (! dev)
     grub_util_error ("%s", grub_errmsg);
 
-  if (print == PRINT_PARTMAP)
+  if (types & PRINT_PARTMAP)
     {
       if (dev->disk->partition == NULL)
         grub_util_error ("Cannot detect partition map for %s", drive_name);
@@ -119,14 +134,16 @@
         printf ("sun\n");
       else
         grub_util_error ("Unknown partition map %s", dev->disk->partition->partmap->name);
-      goto end;
     }
 
-  fs = grub_fs_probe (dev);
-  if (! fs)
-    grub_util_error ("%s", grub_errmsg);
+  if (types & PRINT_FS)
+    {
+      fs = grub_fs_probe (dev);
+      if (! fs)
+        grub_util_error ("%s", grub_errmsg);
 
-  printf ("%s\n", fs->name);
+      printf ("%s\n", fs->name);
+    }
   
   grub_device_close (dev);
 
@@ -159,8 +176,9 @@
 Probe device information for a given path.\n\
 \n\
   -m, --device-map=FILE     use FILE as the device map [default=%s]\n\
-  -t, --target=(fs|drive|device|partmap)\n\
-                            print filesystem module, GRUB drive, system device or partition map module [default=fs]\n\
+  -t, --target=(fs|drive|prefix|device|partmap|all)\n\
+                            print filesystem module, GRUB drive, path prefix, system device\n\
+                            or partition map module [default=fs]\n\
   -h, --help                display this message and exit\n\
   -V, --version             print version information and exit\n\
   -v, --verbose             print verbose messages\n\
@@ -175,6 +193,7 @@
 int
 main (int argc, char *argv[])
 {
+  unsigned types = 0;
   char *dev_map = 0;
   char *path;
   
@@ -199,13 +218,17 @@
 
 	  case 't':
 	    if (!strcmp (optarg, "fs"))
-	      print = PRINT_FS;
+	      types |= PRINT_FS;
 	    else if (!strcmp (optarg, "drive"))
-	      print = PRINT_DRIVE;
+	      types |= PRINT_DRIVE;
+	    else if (!strcmp (optarg, "prefix"))
+	      types |= PRINT_PREFIX;
 	    else if (!strcmp (optarg, "device"))
-	      print = PRINT_DEVICE;
+	      types |= PRINT_DEVICE;
 	    else if (!strcmp (optarg, "partmap"))
-	      print = PRINT_PARTMAP;
+	      types |= PRINT_PARTMAP;
+	    else if (!strcmp (optarg, "all"))
+	      types = ~0;
 	    else
 	      usage (1);
 	    break;
@@ -227,6 +250,8 @@
 	    break;
 	  }
     }
+  if (types == 0)
+    types = PRINT_FS;
 
   /* Obtain PATH.  */
   if (optind >= argc)
@@ -250,7 +275,7 @@
   grub_init_all ();
 
   /* Do it.  */
-  probe (path);
+  probe (path, types);
   
   /* Free resources.  */
   grub_fini_all ();

Re: [PATCH] grub-probe -t prefix, -t all

[Robert Millan]

On Mon, Oct 29, 2007 at 09:20:00PM +0100, Christian Franke wrote:
> This patch adds '-t prefix', '-t all', and allows multiple -t options to 
> be specified.
> 
> '-t prefix' was initially added to test grub_get_prefix() on Cygwin, but 
> may be useful in scripts.

What does -t prefix do?  Is it the same as
make_system_path_relative_to_its_root () ?  Sounds like I screwed up then :-/

> +#define PRINT_FS	0x01
> +#define PRINT_DRIVE	0x02
> +#define PRINT_PREFIX	0x04
> +#define PRINT_DEVICE	0x08
> +#define PRINT_PARTMAP	0x10

In general, GRUB bitmasks are declared as (1 << 0), (1 << 1), etc.  It'd be
nice to keep this consistent.
 
>    drive_name = grub_util_get_grub_dev (device_name);
>    if (! drive_name)
>      grub_util_error ("cannot find a GRUB drive for %s.\n", device_name);

This check (and possibly others) is an abort condition.  Before your change,
if we reached this point we knew we wanted to abort, but this may not be
true anymore.  Did you check this?

> +  -t, --target=(fs|drive|prefix|device|partmap|all)\n\
> +                            print filesystem module, GRUB drive, path prefix, system device\n\
> +                            or partition map module [default=fs]\n\

Uhm what's the purpose of `-t all' ?  grub-probe is only intended to be
called by scripts.

-- 
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] grub-probe -t prefix, -t all

[Christian Franke]

Robert Millan wrote:
> On Mon, Oct 29, 2007 at 09:20:00PM +0100, Christian Franke wrote:
>   
>> This patch adds '-t prefix', '-t all', and allows multiple -t options to 
>> be specified.
>>
>> '-t prefix' was initially added to test grub_get_prefix() on Cygwin, but 
>> may be useful in scripts.
>>     
>
> What does -t prefix do?  Is it the same as
> make_system_path_relative_to_its_root () ?  Sounds like I screwed up then :-/
>
>   

print_result_of_grub_get_grefix ()

or

make_path_relative_to_its_mount_point_and_then_absolute ()

or

please_tell_me_which_prefix_grub_setup_will_patch_into_kernel (:-)


>> +#define PRINT_FS	0x01
>> +#define PRINT_DRIVE	0x02
>> +#define PRINT_PREFIX	0x04
>> +#define PRINT_DEVICE	0x08
>> +#define PRINT_PARTMAP	0x10
>>     
>
> In general, GRUB bitmasks are declared as (1 << 0), (1 << 1), etc.  It'd be
> nice to keep this consistent.
>   

OK.


>  
>   
>>    drive_name = grub_util_get_grub_dev (device_name);
>>    if (! drive_name)
>>      grub_util_error ("cannot find a GRUB drive for %s.\n", device_name);
>>     
>
> This check (and possibly others) is an abort condition.  Before your change,
> if we reached this point we knew we wanted to abort, but this may not be
> true anymore.  Did you check this?
>
>   

No. Yes, logic of grub-probe is subject to improve.


>> +  -t, --target=(fs|drive|prefix|device|partmap|all)\n\
>> +                            print filesystem module, GRUB drive, path prefix, system device\n\
>> +                            or partition map module [default=fs]\n\
>>     
>
> Uhm what's the purpose of `-t all' ?  grub-probe is only intended to be
> called by scripts.
>
>   

Convenient testing all values for non-scripts (e.g. humans:-)
Not really necessary, but with the bitmask approach it comes for free.

Christian

Re: [PATCH] grub-probe -t prefix, -t all

[Robert Millan]

On Fri, Nov 09, 2007 at 10:42:31PM +0100, Christian Franke wrote:
> Robert Millan wrote:
> >On Mon, Oct 29, 2007 at 09:20:00PM +0100, Christian Franke wrote:
> >  
> >>This patch adds '-t prefix', '-t all', and allows multiple -t options to 
> >>be specified.
> >>
> >>'-t prefix' was initially added to test grub_get_prefix() on Cygwin, but 
> >>may be useful in scripts.
> >>    
> >
> >What does -t prefix do?  Is it the same as
> >make_system_path_relative_to_its_root () ?  Sounds like I screwed up then 
> >:-/
> >
> >  
> 
> print_result_of_grub_get_grefix ()
> 
> or
> 
> make_path_relative_to_its_mount_point_and_then_absolute ()
> 
> or
> 
> please_tell_me_which_prefix_grub_setup_will_patch_into_kernel (:-)

Note that make_system_path_relative_to_its_root() actually exists :-/

If you provide an (obviously better) way to do the same thing, maybe you
could also get rid of the other one in your patch?

-- 
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] grub-probe -t prefix, -t all

[Christian Franke]

Robert Millan wrote:
> ...
> Note that make_system_path_relative_to_its_root() actually exists :-/
>
>   

Sorry, I didn't search update-grub_lib.in :-\

Apparently both grub_get_prefix() and 
make_system_path_relative_to_its_root() use the same algorithm. Both do 
not work on Cygwin due to special (multi- to single-root) mount point 
handling.

It is desirable to have only one implementation for this non-trivial 
function. A native C implementation is needed for grub-setup, therefore 
"grub-probe -t prefix" makes sense for scripts.


> If you provide an (obviously better) way to do the same thing, maybe you
> could also get rid of the other one in your patch?
>
>   

Yes.

Christian