[PATCH] menu: Ignore prompt variable if timeout is != 0

[Traut Manuel LCPF-CH]

Hi,

> thank you for your reply! 
thanks for taking care on this somehow lost patch

> On 7/18/23 23:08, Jonas Karlman wrote:
>> On 2023-07-14 09:36, FUKAUMI Naoki wrote:
>>> From: Manuel Traut <manuel.traut at mt.com>
>>>
>>> Since 739e8361f3fe78038251216df6096a32bc2d5839, a system with the 
>>> following /boot/extlinux/extlinux.conf (which sets timeout to 50) 
>>> immediately boots the first entry in the config without displaying a 
>>> boot menu.  According to the description, that should only happen if 
>>> both prompt and timeout are set to zero in the config, but it also happens with timeout set to a non-zero value.
>>>
>>> Reported-by: Karsten Merker <merker at debian.org>
>>> Signed-off-by: Manuel Traut <manuel.traut at mt.com>
>>> ---
>>>   common/menu.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/common/menu.c b/common/menu.c index 
>>> 8fe00965c0..8eabcccc87 100644
>>> --- a/common/menu.c
>>> +++ b/common/menu.c
>>> @@ -277,6 +277,9 @@ int menu_get_choice(struct menu *m, void **choice)
>>>   	if (!m->item_cnt)
>>>   		return -ENOENT;
>>>   
>>> +	if (m->timeout)
>>> +		return menu_interactive_choice(m, choice);
>> 
>> This should not be needed, if the user wants to prompt the menu there 
>> is the PROMPT keyword that can be used in extlinux.conf, e.g.:
>> 
>>    PROMPT 1
>>    TIMEOUT 50
>> 
>> See https://wiki.archlinux.org/title/Syslinux#Boot_prompt
>> 
>> That should set pxe cfg->prompt = 1 and that in turn menu m->prompt = 1.
>
> https://source.denx.de/u-boot/u-boot/-/blob/master/common/menu.c#L346-351
>
> this description is unclear for me if (timeout > 0) && (prompt == 0)

This is my current understanding after reading the description multiple times:

            | timeout == 0 | timeout > 0
------------+--------------+-----------------------------------------
prompt == 0 | boot default | wait for timeout or user interrupt
------------+--------------+-----------------------------------------
prompt != 0 | ask user     | ask user

so for (timeout > 0) && (prompt == 0) I would expect to boot the default choice immediately.

Without reading the documentation I would expect
PROMPT 1
TIMEOUT 50
to show a prompt for 5 seconds and afterwards boot the default choice.
This also matches the current implantation and the explanation in the arch wiki.

The current implementation, behaves like this:

            | timeout == 0 | timeout > 0
------------+--------------+-----------------------------------------
prompt == 0 | boot default | boot default
------------+--------------+-----------------------------------------
prompt != 0 | ask user     | wait for timeout or user input

The patch under discussion considers a configuration of:
PROMPT 0
TIMEOUT 50
that currently is booting the default target immediately.
This clearly does not match the description.

The patch would change the behavior like this:

            | timeout == 0 | timeout > 0
------------+--------------+-----------------------------------------
prompt == 0 | boot default | wait for timeout or user input
------------+--------------+-----------------------------------------
prompt != 0 | ask user     | wait for timeout or user input

It does not match the description regarding:
"prompt - If 1, the user will be prompted for input regardless of the value of timeout"

If I write a configuration like:
PROMPT 1
TIMEOUT 50
I would expect to get a prompt for 5 seconds, than boot the default target.

I can update the patch to include changing the documentation along with table above as a comment to make it easier understandable.
Or shall I update the patch to match the behavior described in the first table?
Just let me know..

Regards
Manuel