On 2/13/23 06:50, Masahisa Kojima wrote:
On Fri, 10 Feb 2023 at 20:57, Heinrich Schuchardt <xypron.g...@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)?

Best regards

Heinrich


Regards,
Masahisa Kojima


Best regards

Heinrich


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


Reply via email to