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