Re: [PATCH 4/5] eficonfig: include EFI_STATUS string in error message

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.

Best regards

Heinrich

Signed-off-by: Masahisa Kojima masahisa.kojima@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);