| Message ID | 20210115011931.5076-1-klaus@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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.
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 :)
Cheers,
Jeremy
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
On 1/20/2021 4:12 PM, Klaus Heinrich Kiwi wrote: > > > 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.. ... sets the linux_gfx_mode to 'keep'
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; case 's': /* -s: return true if file exists and has non-zero size */ result = !path ? false : statbuf.st_size > 0; @@ -336,7 +341,7 @@ static bool builtin_test_op(struct grub2_script *script, return strlen(a1) != 0; } - if (!strcmp(op, "-s") || !strcmp(op, "-f")) { + if (!strcmp(op, "-s") || !strcmp(op, "-f") || !(strcmp(op, "-e"))) { *consumed = 2; return builtin_test_op_file(script, op[1], a1); } 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 @@ -19,13 +19,13 @@ void run_test(struct parser_test *test) check_unresolved_resource(opt->boot_image); check_unresolved_resource(opt->initrd); check_name(opt, "Kubuntu GNU/Linux"); - check_args(opt, "root=UUID=29beca39-9181-4780-bbb2-ab5d4be59aaf ro quiet splash "); + check_args(opt, "root=UUID=29beca39-9181-4780-bbb2-ab5d4be59aaf ro quiet splash vt.handoff=7"); opt = get_boot_option(ctx, 1); check_unresolved_resource(opt->boot_image); check_unresolved_resource(opt->initrd); check_name(opt, "Kubuntu GNU/Linux, with Linux 3.8.0-19-generic"); - check_args(opt, "root=UUID=29beca39-9181-4780-bbb2-ab5d4be59aaf ro quiet splash "); + check_args(opt, "root=UUID=29beca39-9181-4780-bbb2-ab5d4be59aaf ro quiet splash vt.handoff=7"); opt = get_boot_option(ctx, 2); check_name(opt, "Kubuntu GNU/Linux, with Linux 3.8.0-19-generic (recovery mode)");
Grub2 allows a special-case of file test using the '-e' operator, where the path can be empty, and the device existance is tested. E.g.: if [ -e (md/md-boot) ]; then Add the support for testing this condition. This fixes the following RH CoreOS bug: https://bugzilla.redhat.com/show_bug.cgi?id=1915540 Signed-off-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com> --- discover/grub2/builtins.c | 7 ++++++- test/parser/test-grub2-ubuntu-13_04-x86.c | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-)