Re: Bug#461442: detection of other OSes in update-grub

[Robert Millan]

On Mon, Feb 04, 2008 at 09:43:58AM +0100, Fabian Greffrath wrote:
> the given argument if it is a block device and returns NULL else. This 
> was necessary, because else you could force grub-probe to print 'foobar' 
> if run as 'grub-probe --target=device --device foobar'.

Why is that a problem?

> --- grub2-1.95+20080201~/util/getroot.c       2008-01-12 16:11:56.000000000 
> +0100
> +++ grub2-1.95+20080201/util/getroot.c        2008-02-03 21:56:29.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);

Missing space here. ^

> +  else
> +    return 0;
> +}

I think this function could be called from the other part of this file which
performs similar checks (if this functionality is to be kept, that is).

> diff -urNad grub2-1.95+20080201~/util/grub-probe.c 
> grub2-1.95+20080201/util/grub-probe.c
> --- grub2-1.95+20080201~/util/grub-probe.c    2008-01-25 23:33:57.000000000 
> +0100
> +++ grub2-1.95+20080201/util/grub-probe.c     2008-02-03 19:30:50.000000000 
> +0100
> @@ -50,6 +50,7 @@
>  };
>  
>  int print = PRINT_FS;
> +unsigned int argument_is_device = 0;

I know that the call to probe() is not supposed to be reentrant, but I'd
prefer not to break reentrancy if it can be easily avoided;  it is possible
that probe() needs to recurse onto itself in the future (because of RAID/LVM).

> #! /bin/sh -e
> 
> # update-grub helper script.
> # <insert copyright and license blurb here>

Please remember to fix that in later versions of the patch ;-)

>         linux)
>           if [ -x "`which linux-boot-prober 2>/dev/null`" ] ; then
>             LINUXPROBED="`linux-boot-prober ${DEVICE} | tr ' ' '|' | paste -s 
> -d ' '`"
>           fi

Is it possible to share code with 10_linux.in here?

>           if [ -n "${LINUXPROBED}" ] ; then
>             for LINUX in ${LINUXPROBED} ; do
>               LROOT="`echo ${LINUX} | cut -d ':' -f 1`"
>               LBOOT="`echo ${LINUX} | cut -d ':' -f 2`"
>               LLABEL="`echo ${LINUX} | cut -d ':' -f 3 | tr '|' ' '`"
>               LKERNEL="`echo ${LINUX} | cut -d ':' -f 4`"
>               LINITRD="`echo ${LINUX} | cut -d ':' -f 5`"
>               LPARAMS="`echo ${LINUX} | cut -d ':' -f 6 | tr '|' ' '`"

Maybe this can be simplified with "echo something | read a b c d" feature?

-- 
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 /.)