default menuentry matching similar entries is broken

default menuentry matching similar entries is broken

[Seth Goldberg]

Hi,

  This code in menu.c::menuentry_eq() looks wrong:

{
  const char *ptr1, *ptr2;
  ptr1 = title;
  ptr2 = spec;
  while (1)
    {
      if (*ptr2 == '>' && ptr2[1] != '>' && *ptr1 == 0)
        return 1;
      if (*ptr2 == '>' && ptr2[1] != '>')
        return 0;
      if (*ptr2 == '>')
        ptr2++;
      if (*ptr1 != *ptr2)
        return 0;
      if (*ptr1 == 0)   <----
        return 1;
      ptr1++;
      ptr2++;
    }
}


  Specifically, if there are two menuentries that differ by adding characters, this function will match the wrong one, i.e.:

set default='entry a with more words'

menuentry "entry a" {
}

menuentry "entry a with more words" {
}

 The default that will be booted will the the first one, which is clearly wrong.

  Why was the *ptr1 == 0 code added?  Also, what's the deal with the '>' characters?  What are they supposed to do? I can't seem to find the use of '>'  documented anywhere.

 Thanks,
 --S

Re: default menuentry matching similar entries is broken

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 03.03.2012 08:33, Seth Goldberg wrote:
> Hi,
>
>    This code in menu.c::menuentry_eq() looks wrong:
>
> {
>    const char *ptr1, *ptr2;
>    ptr1 = title;
>    ptr2 = spec;
>    while (1)
>      {
>        if (*ptr2 == '>'&&  ptr2[1] != '>'&&  *ptr1 == 0)
>          return 1;
>        if (*ptr2 == '>'&&  ptr2[1] != '>')
>          return 0;
>        if (*ptr2 == '>')
>          ptr2++;
>        if (*ptr1 != *ptr2)
>          return 0;
>        if (*ptr1 == 0)<----
>          return 1;
>        ptr1++;
>        ptr2++;
>      }
> }
>
>
>    Specifically, if there are two menuentries that differ by adding characters, this function will match the wrong one, i.e.:
Have you actually tested this? Since from the code readin just before it 
we have:

       if (*ptr1 != *ptr2)
         return 0;

So the code you point at is trigered only if both *ptr1 and *ptr2 are 0
>   Also, what's the deal with the '>' characters?  What are they supposed to do? I can't seem to find the use of'>'  documented anywhere.
It's for submenus:
"submenu>subsubmenu>entry"
>
>   Thanks,
>   --S
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

Re: default menuentry matching similar entries is broken

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 03.03.2012 15:02, Seth Goldberg wrote:
> Sigh.  I thought I tracked it down to this code, but it was late.  I'll double-check.  The buggy behavior is as I described -- this just may not be the actual root cause.  BTW, with the code below, does this mean that having '>' characters in the title of menuentries is forbidden?
No, just that you have to escape it with double >. E.g.
default="weird>>submenu>entry"

>   Thanks,
>   --S
>
>
> On Mar 3, 2012, at 3:09 AM, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>
>> On 03.03.2012 08:33, Seth Goldberg wrote:
>>> Hi,
>>>
>>>    This code in menu.c::menuentry_eq() looks wrong:
>>>
>>> {
>>>    const char *ptr1, *ptr2;
>>>    ptr1 = title;
>>>    ptr2 = spec;
>>>    while (1)
>>>      {
>>>        if (*ptr2 == '>'&&   ptr2[1] != '>'&&   *ptr1 == 0)
>>>          return 1;
>>>        if (*ptr2 == '>'&&   ptr2[1] != '>')
>>>          return 0;
>>>        if (*ptr2 == '>')
>>>          ptr2++;
>>>        if (*ptr1 != *ptr2)
>>>          return 0;
>>>        if (*ptr1 == 0)<----
>>>          return 1;
>>>        ptr1++;
>>>        ptr2++;
>>>      }
>>> }
>>>
>>>
>>>    Specifically, if there are two menuentries that differ by adding characters, this function will match the wrong one, i.e.:
>> Have you actually tested this? Since from the code readin just before it we have:
>>
>>       if (*ptr1 != *ptr2)
>>         return 0;
>>
>> So the code you point at is trigered only if both *ptr1 and *ptr2 are 0
>>>   Also, what's the deal with the '>' characters?  What are they supposed to do? I can't seem to find the use of'>'  documented anywhere.
>> It's for submenus:
>> "submenu>subsubmenu>entry"
>>>   Thanks,
>>>   --S
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>> -- 
>> Regards
>> Vladimir 'φ-coder/phcoder' Serbinenko
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

Re: default menuentry matching similar entries is broken

[Andreas Vogel]

> Hi,
>
>   This code in menu.c::menuentry_eq() looks wrong:
>
> {
>   const char *ptr1, *ptr2;
>   ptr1 = title;
>   ptr2 = spec;
>   while (1)
>     {
>       if (*ptr2 == '>' && ptr2[1] != '>' && *ptr1 == 0)
>         return 1;
>       if (*ptr2 == '>' && ptr2[1] != '>')
>         return 0;
>       if (*ptr2 == '>')
>         ptr2++;
>       if (*ptr1 != *ptr2)
>         return 0;
>       if (*ptr1 == 0)   <----
>         return 1;
>       ptr1++;
>       ptr2++;
>     }
> }
>
>
>   Specifically, if there are two menuentries that differ by adding characters, this function will match the wrong one, i.e.:
>
> set default='entry a with more words'
>
> menuentry "entry a" {
> }
>
> menuentry "entry a with more words" {
> }
As far as I understand the code, using submenu references in the default
variable for automatic booting is not working at all.
This is due to the dynamic loading of submenus. When a menu is opened
and run, only the direct menu entries are known. Searching for menu
entries deeper in the menu tree is not done.

Let's consider this scenario:

set default="b>c>c1"

menuentry a { ... }
submenu b {
    submenu c {
        menuentry c1 { ... }
    }
}

When the toplevel menu is run, the code just knows about entries a and b
(entries c and c1 are not know at this moment).
Searching and booting the menu entry according to the default variable
doesn't work. Actually I didn't try this, that's just from code reading
so i might be wrong.

IMHO, right now the menuentry_eq() function is overkill as a normal
strcmp() would be sufficient.

Furthermore it's strange that calling
    menuentry_eq ("a>b", "a>b")
returns 0. I didn't expect this result.


Andreas

Re: default menuentry matching similar entries is broken

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 03.03.2012 19:10, Andreas Vogel wrote:
>> Hi,
>>
>>    This code in menu.c::menuentry_eq() looks wrong:
>>
>> {
>>    const char *ptr1, *ptr2;
>>    ptr1 = title;
>>    ptr2 = spec;
>>    while (1)
>>      {
>>        if (*ptr2 == '>'&&  ptr2[1] != '>'&&  *ptr1 == 0)
>>          return 1;
>>        if (*ptr2 == '>'&&  ptr2[1] != '>')
>>          return 0;
>>        if (*ptr2 == '>')
>>          ptr2++;
>>        if (*ptr1 != *ptr2)
>>          return 0;
>>        if (*ptr1 == 0)<----
>>          return 1;
>>        ptr1++;
>>        ptr2++;
>>      }
>> }
>>
>>
>>    Specifically, if there are two menuentries that differ by adding characters, this function will match the wrong one, i.e.:
>>
>> set default='entry a with more words'
>>
>> menuentry "entry a" {
>> }
>>
>> menuentry "entry a with more words" {
>> }
> As far as I understand the code, using submenu references in the default
> variable for automatic booting is not working at all.
> This is due to the dynamic loading of submenus. When a menu is opened
> and run, only the direct menu entries are known. Searching for menu
> entries deeper in the menu tree is not done.
No need. When timeout is reached the submenu is executed and first 
element is discarded and the rest is searched for again.

-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

Re: default menuentry matching similar entries is broken

[Andreas Vogel]

Am 03.03.2012 19:20, schrieb Vladimir 'φ-coder/phcoder' Serbinenko:
> On 03.03.2012 19:10, Andreas Vogel wrote:
>>> Hi,
>>>
>>>    This code in menu.c::menuentry_eq() looks wrong:
>>>
>>> {
>>>    const char *ptr1, *ptr2;
>>>    ptr1 = title;
>>>    ptr2 = spec;
>>>    while (1)
>>>      {
>>>        if (*ptr2 == '>'&&  ptr2[1] != '>'&&  *ptr1 == 0)
>>>          return 1;
>>>        if (*ptr2 == '>'&&  ptr2[1] != '>')
>>>          return 0;
>>>        if (*ptr2 == '>')
>>>          ptr2++;
>>>        if (*ptr1 != *ptr2)
>>>          return 0;
>>>        if (*ptr1 == 0)<----
>>>          return 1;
>>>        ptr1++;
>>>        ptr2++;
>>>      }
>>> }
>>>
>>>
>>>    Specifically, if there are two menuentries that differ by adding
>>> characters, this function will match the wrong one, i.e.:
>>>
>>> set default='entry a with more words'
>>>
>>> menuentry "entry a" {
>>> }
>>>
>>> menuentry "entry a with more words" {
>>> }
>> As far as I understand the code, using submenu references in the default
>> variable for automatic booting is not working at all.
>> This is due to the dynamic loading of submenus. When a menu is opened
>> and run, only the direct menu entries are known. Searching for menu
>> entries deeper in the menu tree is not done.
> No need. When timeout is reached the submenu is executed and first
> element is discarded and the rest is searched for again.
>

I'm sorry... and you're right! Actually the code was too obfuscated for
me...

BTW, one of the features which are missing and about what i read in
different forums is that it's not possible to reference entries in the
menu tree by number, e.g. "0>2>4>2". By reading the code i would say
it's not possible, but after my failure right now, I better ask.... :-)

Re: default menuentry matching similar entries is broken

[Vladimir 'φ-coder/phcoder' Serbinenko]

>  BTW, one of the features which are missing and about what i read in
>  different forums is that it's not possible to reference entries in the
>  menu tree by number, e.g. "0>2>4>2".

This should work but isn't recommended if it doesn't please supply a 
testcase or a fix.


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

Re: default menuentry matching similar entries is broken

[Andreas Vogel]

>>  BTW, one of the features which are missing and about what i read in
>>  different forums is that it's not possible to reference entries in the
>>  menu tree by number, e.g. "0>2>4>2".
>
> This should work but isn't recommended if it doesn't please supply a
> testcase or a fix.
>
>

Just to let you know: I created a quite complicated testcase with 4
submenus and it works!