[PATCH v2 2/2] discover/grub2: Add the '-e' test support

[Klaus Heinrich Kiwi]

On 1/20/2021 2:08 AM, Jeremy Kerr wrote:
> Hi Klaus,
> 
> diff --git a/discover/grub2/builtins.c b/discover/grub2/builtins.c
> index 31cbe0e..291bb03 100644
> --- a/discover/grub2/builtins.c
> +++ b/discover/grub2/builtins.c
> @@ -245,6 +245,11 @@ static bool builtin_test_op_file(struct
> grub2_script *script, char op,
>                  return false;
>   
>          switch (op) {
> +       case 'e':
> +               /* -e: for grub, a special case is testing for the
> device
> +                * presence itself (e.g. allows null file). */
> +               result = true;
> +               break;
> 
> I would suggest moving this earlier, so we avoid the stat (which we
> don't care about for '-e'). Then you can handle the path = NULL case in
> one conditional, rather than in each 'op' branch.

actually we do care for stat for '-e'.. it would fail (and the function woul
prematurely return false) if the file / device doesn't exist.

We need that because '-e' can still be used to check for files' existance
(in addition to just devices)

> 
> diff --git a/test/parser/test-grub2-ubuntu-13_04-x86.c
> b/test/parser/test-grub2-ubuntu-13_04-x86.c
> index 2f9aefd..785781a 100644
> --- a/test/parser/test-grub2-ubuntu-13_04-x86.c
> +++ b/test/parser/test-grub2-ubuntu-13_04-x86.c
> 
> This looks unrelated too :)


It's actually related.. the relevant test file has this:

if [ "${recordfail}" != 1 ]; then
   if [ -e ${prefix}/gfxblacklist.txt ]; then
     if hwmatch ${prefix}/gfxblacklist.txt 3; then
       if [ ${match} = 0 ]; then
         set linux_gfx_mode=keep
       else
         set linux_gfx_mode=text
       fi
     else
       set linux_gfx_mode=text
     fi
   else
     set linux_gfx_mode=keep
   fi
else
   set linux_gfx_mode=text
fi
export linux_gfx_mode


What was happening is that the parser was skipping the entire statement
when hitting the -e, but since now we support it, it is going with
the else statement, which sets linux_gfx_mode=text..

And then there's a function:

function gfxmode {
         set gfxpayload="${1}"
         if [ "${1}" = "keep" ]; then
                 set vt_handoff=vt.handoff=7
         else
                 set vt_handoff=
         fi
}

which is in turn called inside the menuentries:

menuentry 'Kubuntu GNU/Linux' --class kubuntu --class gnu-linux --class gnu --class os $menuentry_id_option 'gnulinux-simple-29beca39-9181-4780-bbb2-ab5d4be59aaf' {
recordfail
         load_video
         gfxmode $linux_gfx_mode
         insmod gzio
         insmod part_msdos
         insmod ext2
         set root='hd0,msdos1'
         if [ x$feature_platform_search_hint = xy ]; then
           search --no-floppy --fs-uuid --set=root --hint-bios=hd0,msdos1 --hint-efi=hd0,msdos1 --hint-baremetal=ahci0,msdos1  29beca39-9181-4780-bbb2-ab5d4be59aaf
         else
           search --no-floppy --fs-uuid --set=root 29beca39-9181-4780-bbb2-ab5d4be59aaf
         fi
         linux   /boot/vmlinuz-3.8.0-19-generic root=UUID=29beca39-9181-4780-bbb2-ab5d4be59aaf ro   quiet splash $vt_handoff

thus $vt_handoff should be replaced by 'vt.handoff=7' because 'gfxmode == keep'


> Cheers,
> 
> 
> Jeremy
> 
> 

Thanks!

  -Klaus

-- 
Klaus Heinrich Kiwi <klaus at linux.vnet.ibm.com>