[PATCH 4/5] eficonfig: include EFI_STATUS string in error message
Heinrich Schuchardt
xypron.glpk at gmx.de
Mon Feb 13 10:46:02 CET 2023
On 2/13/23 10:42, Masahisa Kojima wrote:
> Hi Heinrich,
>
> On Mon, 13 Feb 2023 at 16:44, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> On 2/13/23 06:50, Masahisa Kojima wrote:
>>> On Fri, 10 Feb 2023 at 20:57, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>>>
>>>> On 2/2/23 10:24, Masahisa Kojima wrote:
>>>>> Current eficonfig_print_msg() does not show the return
>>>>> value of EFI Boot/Runtime Services when the service call fails.
>>>>> With this commit, user can know EFI_STATUS in the error message.
>>>>
>>>> Why do we need function eficonfig_print_msg()?
>>>>
>>>> I cannot see why the printing only parameter msg with log_err() should
>>>> not be good enough.
>>>
>>> ANSI_CLEAR_CONSOLE is sent before drawing the menu. I think it is
>>> difficult for user
>>> to know some error occurs by the user operation, user needs scroll up
>>> to see the error message
>>> when we use log_err(
>> Understood. But why do we need the status value (or with this patch the
>> long text for the status value)?
>
> At first, I planned to add additional error messages specific to some
> status value, but it will increase the eficonfig_print_msg() calls.
Which message remains unclear without the extra information?
> Instead of adding eficonfig_print_msg() calls,
> I think printing the status value(or text for the status value) will reduce the
> code size eventually.
> But printing the status code will not much help user to understand
> what the error cause is.
>
> Thanks,
> Masahisa Kojima
>
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> Regards,
>>> Masahisa Kojima
>>>
>>>>
>>>> Best regards
>>>>
>>>> Heinrich
>>>>
>>>>>
>>>>> Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
>>>>> ---
>>>>> cmd/eficonfig.c | 95 +++++++++++++++++++++++++++++++++++++------
>>>>> cmd/eficonfig_sbkey.c | 16 ++++----
>>>>> include/efi_config.h | 2 +-
>>>>> 3 files changed, 93 insertions(+), 20 deletions(-)
>>>>>
>>>>> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
>>>>> index 0a17b8cf34..b0c8637676 100644
>>>>> --- a/cmd/eficonfig.c
>>>>> +++ b/cmd/eficonfig.c
>>>>> @@ -151,19 +151,90 @@ static void eficonfig_menu_adjust(struct efimenu *efi_menu, bool add)
>>>>> #define eficonfig_menu_up(_a) eficonfig_menu_adjust(_a, false)
>>>>> #define eficonfig_menu_down(_a) eficonfig_menu_adjust(_a, true)
>>>>>
>>>>> +struct efi_status_str {
>>>>> + efi_status_t status;
>>>>> + char *str;
>>>>> +};
>>>>> +
>>>>> +static const struct efi_status_str status_str_table[] = {
>>>>> + {EFI_LOAD_ERROR, "Load Error"},
>>>>> + {EFI_INVALID_PARAMETER, "Invalid Parameter"},
>>>>> + {EFI_UNSUPPORTED, "Unsupported"},
>>>>> + {EFI_BAD_BUFFER_SIZE, "Bad Buffer Size"},
>>>>> + {EFI_BUFFER_TOO_SMALL, "Buffer Too Small"},
>>>>> + {EFI_NOT_READY, "Not Ready"},
>>>>> + {EFI_DEVICE_ERROR, "Device Error"},
>>>>> + {EFI_WRITE_PROTECTED, "Write Protected"},
>>>>> + {EFI_OUT_OF_RESOURCES, "Out of Resources"},
>>>>> + {EFI_VOLUME_CORRUPTED, "Volume Corrupted"},
>>>>> + {EFI_VOLUME_FULL, "Volume Full"},
>>>>> + {EFI_NO_MEDIA, "No Media"},
>>>>> + {EFI_MEDIA_CHANGED, "Media Changed"},
>>>>> + {EFI_NOT_FOUND, "Not Found"},
>>>>> + {EFI_ACCESS_DENIED, "Access Denied"},
>>>>> + {EFI_NO_RESPONSE, "No Response"},
>>>>> + {EFI_NO_MAPPING, "No Mapping"},
>>>>> + {EFI_TIMEOUT, "Timeout"},
>>>>> + {EFI_NOT_STARTED, "Not Started"},
>>>>> + {EFI_ALREADY_STARTED, "Already Started"},
>>>>> + {EFI_ABORTED, "Aborted"},
>>>>> + {EFI_ICMP_ERROR, "ICMP Error"},
>>>>> + {EFI_TFTP_ERROR, "TFTP Error"},
>>>>> + {EFI_PROTOCOL_ERROR, "Protocol Error"},
>>>>> + {EFI_INCOMPATIBLE_VERSION, "Incompatible Version"},
>>>>> + {EFI_SECURITY_VIOLATION, "Security Violation"},
>>>>> + {EFI_CRC_ERROR, "CRC Error"},
>>>>> + {EFI_END_OF_MEDIA, "End of Media"},
>>>>> + {EFI_END_OF_FILE, "End of File"},
>>>>> + {EFI_INVALID_LANGUAGE, "Invalid Language"},
>>>>> + {EFI_COMPROMISED_DATA, "Compromised Data"},
>>>>> + {EFI_IP_ADDRESS_CONFLICT, "IP Address Conflict"},
>>>>> + {EFI_HTTP_ERROR, "HTTP Error"},
>>>>> + {EFI_WARN_UNKNOWN_GLYPH, "Warn Unknown Glyph"},
>>>>> + {EFI_WARN_DELETE_FAILURE, "Warn Delete Failure"},
>>>>> + {EFI_WARN_WRITE_FAILURE, "Warn Write Failure"},
>>>>> + {EFI_WARN_BUFFER_TOO_SMALL, "Warn Buffer Too Small"},
>>>>> + {EFI_WARN_STALE_DATA, "Warn Stale Data"},
>>>>> + {EFI_WARN_FILE_SYSTEM, "Warn File System"},
>>>>> + {EFI_WARN_RESET_REQUIRED, "Warn Reset Required"},
>>>>> + {0, ""},
>>>>> +};
>>>>> +
>>>>> +/**
>>>>> + * struct get_status_str - get status string
>>>>> + *
>>>>> + * @status: efi_status_t value to covert to string
>>>>> + * Return: pointer to the string
>>>>> + */
>>>>> +static char *get_error_str(efi_status_t status)
>>>>> +{
>>>>> + u32 i;
>>>>> +
>>>>> + for (i = 0; status_str_table[i].status != 0; i++) {
>>>>> + if (status == status_str_table[i].status)
>>>>> + return status_str_table[i].str;
>>>>> + }
>>>>> + return status_str_table[i].str;
>>>>> +}
>>>>> +
>>>>> /**
>>>>> * eficonfig_print_msg() - print message
>>>>> *
>>>>> * display the message to the user, user proceeds the screen
>>>>> * with any key press.
>>>>> *
>>>>> - * @items: pointer to the structure of each menu entry
>>>>> - * @count: the number of menu entry
>>>>> - * @menu_header: pointer to the menu header string
>>>>> - * Return: status code
>>>>> + * @msg: pointer to the error message
>>>>> + * @status: efi status code, set 0 if no status string output
>>>>> */
>>>>> -void eficonfig_print_msg(char *msg)
>>>>> +void eficonfig_print_msg(const char *msg, efi_status_t status)
>>>>> {
>>>>> + char str[128];
>>>>> +
>>>>> + if (status == 0)
>>>>> + snprintf(str, sizeof(str), "%s", msg);
>>>>> + else
>>>>> + snprintf(str, sizeof(str), "%s (%s)", msg, get_error_str(status));
>>>>> +
>>>>> /* Flush input */
>>>>> while (tstc())
>>>>> getchar();
>>>>> @@ -171,7 +242,7 @@ void eficonfig_print_msg(char *msg)
>>>>> printf(ANSI_CURSOR_HIDE
>>>>> ANSI_CLEAR_CONSOLE
>>>>> ANSI_CURSOR_POSITION
>>>>> - "%s\n\n Press any key to continue", 3, 4, msg);
>>>>> + "%s\n\n Press any key to continue", 3, 4, str);
>>>>>
>>>>> getchar();
>>>>> }
>>>>> @@ -580,7 +651,7 @@ static efi_status_t eficonfig_file_selected(void *data)
>>>>> new_len = u16_strlen(info->file_info->current_path) +
>>>>> strlen(info->file_name);
>>>>> if (new_len >= EFICONFIG_FILE_PATH_MAX) {
>>>>> - eficonfig_print_msg("File path is too long!");
>>>>> + eficonfig_print_msg("File path is too long!", 0);
>>>>> return EFI_INVALID_PARAMETER;
>>>>> }
>>>>> tmp = &info->file_info->current_path[u16_strlen(info->file_info->current_path)];
>>>>> @@ -626,7 +697,7 @@ static efi_status_t eficonfig_select_volume(struct eficonfig_select_file_info *f
>>>>> ret = efi_locate_handle_buffer_int(BY_PROTOCOL, &efi_simple_file_system_protocol_guid,
>>>>> NULL, &count, (efi_handle_t **)&volume_handles);
>>>>> if (ret != EFI_SUCCESS) {
>>>>> - eficonfig_print_msg("No block device found!");
>>>>> + eficonfig_print_msg("No block device found!", ret);
>>>>> return ret;
>>>>> }
>>>>>
>>>>> @@ -850,7 +921,7 @@ static efi_status_t eficonfig_show_file_selection(struct eficonfig_select_file_i
>>>>> ret = EFI_CALL(root->open(root, &f, file_info->current_path,
>>>>> EFI_FILE_MODE_READ, 0));
>>>>> if (ret != EFI_SUCCESS) {
>>>>> - eficonfig_print_msg("Reading volume failed!");
>>>>> + eficonfig_print_msg("Reading volume failed!", ret);
>>>>> free(efi_menu);
>>>>> ret = EFI_ABORTED;
>>>>> goto out;
>>>>> @@ -990,12 +1061,12 @@ static efi_status_t eficonfig_boot_edit_save(void *data)
>>>>> struct eficonfig_boot_option *bo = data;
>>>>>
>>>>> if (u16_strlen(bo->description) == 0) {
>>>>> - eficonfig_print_msg("Boot Description is empty!");
>>>>> + eficonfig_print_msg("Boot Description is empty!", 0);
>>>>> bo->edit_completed = false;
>>>>> return EFI_NOT_READY;
>>>>> }
>>>>> if (u16_strlen(bo->file_info.current_path) == 0) {
>>>>> - eficonfig_print_msg("File is not selected!");
>>>>> + eficonfig_print_msg("File is not selected!", 0);
>>>>> bo->edit_completed = false;
>>>>> return EFI_NOT_READY;
>>>>> }
>>>>> @@ -2658,7 +2729,7 @@ static efi_status_t eficonfig_init(void)
>>>>> avail_row = rows - (EFICONFIG_MENU_HEADER_ROW_NUM +
>>>>> EFICONFIG_MENU_DESC_ROW_NUM);
>>>>> if (avail_row <= 0) {
>>>>> - eficonfig_print_msg("Console size is too small!");
>>>>> + eficonfig_print_msg("Console size is too small!", 0);
>>>>> return EFI_INVALID_PARAMETER;
>>>>> }
>>>>> /* TODO: Should we check the minimum column size? */
>>>>> diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c
>>>>> index caca27495e..7ae1953567 100644
>>>>> --- a/cmd/eficonfig_sbkey.c
>>>>> +++ b/cmd/eficonfig_sbkey.c
>>>>> @@ -150,7 +150,7 @@ static efi_status_t eficonfig_process_enroll_key(void *data)
>>>>> free(buf);
>>>>>
>>>>> if (!size) {
>>>>> - eficonfig_print_msg("ERROR! File is empty.");
>>>>> + eficonfig_print_msg("ERROR! File is empty.", 0);
>>>>> ret = EFI_INVALID_PARAMETER;
>>>>> goto out;
>>>>> }
>>>>> @@ -163,11 +163,13 @@ static efi_status_t eficonfig_process_enroll_key(void *data)
>>>>>
>>>>> ret = EFI_CALL(f->read(f, &size, buf));
>>>>> if (ret != EFI_SUCCESS) {
>>>>> - eficonfig_print_msg("ERROR! Failed to read file.");
>>>>> + eficonfig_print_msg("ERROR! Failed to read file.", ret);
>>>>> goto out;
>>>>> }
>>>>> if (!file_have_auth_header(buf, size)) {
>>>>> - eficonfig_print_msg("ERROR! Invalid file format. Only .auth variables is allowed.");
>>>>> + eficonfig_print_msg(
>>>>> + "ERROR! Invalid file format. Only .auth variables is allowed.",
>>>>> + 0);
>>>>> ret = EFI_INVALID_PARAMETER;
>>>>> goto out;
>>>>> }
>>>>> @@ -175,7 +177,7 @@ static efi_status_t eficonfig_process_enroll_key(void *data)
>>>>> ret = file_is_null_key((struct efi_variable_authentication_2 *)buf,
>>>>> size, &null_key);
>>>>> if (ret != EFI_SUCCESS) {
>>>>> - eficonfig_print_msg("ERROR! Invalid file format.");
>>>>> + eficonfig_print_msg("ERROR! Invalid file format.", ret);
>>>>> goto out;
>>>>> }
>>>>>
>>>>> @@ -202,7 +204,7 @@ static efi_status_t eficonfig_process_enroll_key(void *data)
>>>>> ret = efi_set_variable_int((u16 *)data, efi_auth_var_get_guid((u16 *)data),
>>>>> attr, size, buf, false);
>>>>> if (ret != EFI_SUCCESS)
>>>>> - eficonfig_print_msg("ERROR! Failed to update signature database");
>>>>> + eficonfig_print_msg("ERROR! Failed to update signature database", ret);
>>>>>
>>>>> out:
>>>>> free(file_info.current_path);
>>>>> @@ -283,7 +285,7 @@ static efi_status_t eficonfig_process_show_siglist(void *data)
>>>>> break;
>>>>> }
>>>>> default:
>>>>> - eficonfig_print_msg("ERROR! Unsupported format.");
>>>>> + eficonfig_print_msg("ERROR! Unsupported format.", 0);
>>>>> return EFI_INVALID_PARAMETER;
>>>>> }
>>>>> }
>>>>> @@ -394,7 +396,7 @@ static efi_status_t enumerate_and_show_signature_database(void *varname)
>>>>>
>>>>> db = efi_get_var(varname, efi_auth_var_get_guid(varname), &db_size);
>>>>> if (!db) {
>>>>> - eficonfig_print_msg("There is no entry in the signature database.");
>>>>> + eficonfig_print_msg("There is no entry in the signature database.", 0);
>>>>> return EFI_NOT_FOUND;
>>>>> }
>>>>>
>>>>> diff --git a/include/efi_config.h b/include/efi_config.h
>>>>> index 01ce9b2b06..19b1686907 100644
>>>>> --- a/include/efi_config.h
>>>>> +++ b/include/efi_config.h
>>>>> @@ -93,7 +93,7 @@ struct eficonfig_select_file_info {
>>>>> bool file_selected;
>>>>> };
>>>>>
>>>>> -void eficonfig_print_msg(char *msg);
>>>>> +void eficonfig_print_msg(const char *msg, efi_status_t status);
>>>>> void eficonfig_print_entry(void *data);
>>>>> void eficonfig_display_statusline(struct menu *m);
>>>>> char *eficonfig_choice_entry(void *data);
>>>>
>>
More information about the U-Boot
mailing list