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

[Jonas Karlman]

On 2023-07-19 10:16, Traut Manuel LCPF-CH wrote:
> 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..

I agree that the documentation is not clear, because this is a common
menu function I would expect the use of timeout and prompt to be
explicit and that each control a single function.

prompt: 0 = do not ask user, 1 = ask user
timeout: 0 = wait indefinitely, > 0 time to wait for input

If anything needs to be done related to how extlinux.conf handles menu
options the menu config should probably be changed in pxe_utils.c and
not in the common menu.c.

My expectations related to extlinux.conf would be:
- No need to prompt or wait for timeout if there is only one bootable
  choice, regardless of PROMPT or TIMEOUT values
- Should not prompt when I have explicitly specified PROMPT 0
- Having multiple bootable choices and a TIMEOUT > 0 or PROMPT 1 should
  prompt, at least if I have some way to see and control the menu

Regards,
Jonas

> 
> Regards
> Manuel