[PATCH v2 06/11] efi_loader: move console-input helpers to efi_app_common

[Heinrich Schuchardt]

On 6/25/26 10:54, Simon Glass wrote:
> Hi Heinrich,
> 
> On 2026-06-21T08:19:04, Heinrich Schuchardt
> <heinrich.schuchardt at canonical.com> wrote:
>> efi_loader: move console-input helpers to efi_app_common
>>
>> Move the shared console-input helpers to efi_app_common:
>>
>> * efi_drain_input() - drain the console input queue
>> * efi_input_yn()    - read a y/n/ESC key from the user
>> * efi_input()       - read a UTF-16 string from the console
>>
>> The common efi_input_yn() calls efi_drain_input() once at entry,
>> replacing the open-coded cin->reset() calls that each application
>> previously duplicated.
>>
>> dbginfodump and smbiosdump had no efi_drain_input() call in their
>> command loops; add the call so the console queue is drained before
>> each prompt, fixing a latent input-echo artefact.
>>
>> The common state is extended with a cin pointer (initialized by
>> efi_app_init from sys_table->con_in) so the helpers can access the
>> console-input protocol without additional parameters.
>>
>> [...]
>>
>> lib/efi_loader/dbginfodump.c    |  61 ----------------------
>>   lib/efi_loader/dtbdump.c        | 109 ----------------------------------------
>>   lib/efi_loader/efi_app_common.c |  88 ++++++++++++++++++++++++++++++++
>>   lib/efi_loader/efi_app_common.h |  29 +++++++++++
>>   lib/efi_loader/initrddump.c     | 109 ----------------------------------------
>>   lib/efi_loader/smbiosdump.c     | 108 +--------------------------------------
>>   6 files changed, 119 insertions(+), 385 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> thoughts below
> 
>> diff --git a/lib/efi_loader/dbginfodump.c b/lib/efi_loader/dbginfodump.c
>> @@ -277,7 +217,6 @@ efi_status_t EFIAPI efi_main(efi_handle_t image_handle,
>>        handle = image_handle;
>>        systable = systab;
>>        cout = systable->con_out;
>> -     cin = systable->con_in;
>>        bs = systable->boottime;
> 
> The commit message says efi_drain_input() is added to dbginfodump and
> smbiosdump command loops, but only smbiosdump's command_loop() gets it
> - dbginfodump's efi_main() loop does not. On top of that, the old
> dbginfodump efi_input() opened with cin->reset(cin, true); the new
> common efi_input() does not drain at entry, so this patch silently
> removes drain-on-input from dbginfodump. Please add efi_drain_input()
> to dbginfodump's command loop too, or update the commit message.
> 
>> diff --git a/lib/efi_loader/efi_app_common.c b/lib/efi_loader/efi_app_common.c
>> @@ -104,3 +106,89 @@ bool starts_with(u16 *string, u16 *keyword)
>> +efi_status_t efi_input(u16 *buffer, efi_uintn_t buffer_size)
>> +{
>> +     struct efi_input_key key = { 0 };
>> +     efi_uintn_t index;
>> +     efi_uintn_t pos = 0;
>> +     u16 outbuf[2] = u" ";
>> +     efi_status_t ret;
>> +
>> +     *buffer = 0;
> 
> efi_input_yn() drains at entry but efi_input() does not - easy to trip
> over later. Either drain inside efi_input() too (matching the old
> dbginfodump/smbiosdump versions), or document in the header that
> callers must drain themselves. What do you think?
> 
> Regards,
> Simon

In the end this is the only usage of efi_input():

lib/efi_loader/efi_app_common.c:359:            ret = efi_input(command, 
CMD_BUFFER_SIZE);

One could argue if draining the console in the command() loop is needed 
or if we want a type ahead buffer.

For input_yn() I think draining is a necessity.

For input() I could imagine future usages where you want to be able to 
paste multiple lines.

The description of efi_status_t efi_input_yn() clearly states that it 
drains the input. While efi_input() does not mention it. So I think the 
documentation is clear.

Thanks for pointing out the discrepancy for dtbdump. Missed that when 
splitting the original single patch.

In the end all apps use the same command loop with efi_drain().

Best regards

Heinrich