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

2023-02-02 Thread Masahisa Kojima
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.

Signed-off-by: Masahisa Kojima 
---
 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_i

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

2023-02-12 Thread Masahisa Kojima
On Fri, 10 Feb 2023 at 20:57, Heinrich Schuchardt  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().

Regards,
Masahisa Kojima

>
> Best regards
>
> Heinrich
>
> >
> > Signed-off-by: Masahisa Kojima 
> > ---
> >   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 efico

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

2023-02-12 Thread Heinrich Schuchardt

On 2/13/23 06:50, Masahisa Kojima wrote:

On Fri, 10 Feb 2023 at 20:57, Heinrich Schuchardt  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 
---
   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
+ snpr

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

2023-02-13 Thread Masahisa Kojima
Hi Heinrich,

On Mon, 13 Feb 2023 at 16:44, Heinrich Schuchardt  wrote:
>
> On 2/13/23 06:50, Masahisa Kojima wrote:
> > On Fri, 10 Feb 2023 at 20:57, Heinrich Schuchardt  
> > 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.
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 
> >>> ---
> >>>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
> >>>

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

2023-02-13 Thread Heinrich Schuchardt

On 2/13/23 10:42, Masahisa Kojima wrote:

Hi Heinrich,

On Mon, 13 Feb 2023 at 16:44, Heinrich Schuchardt  wrote:


On 2/13/23 06:50, Masahisa Kojima wrote:

On Fri, 10 Feb 2023 at 20:57, Heinrich Schuchardt  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 
---
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, 

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

2023-02-13 Thread Masahisa Kojima
On Mon, 13 Feb 2023 at 18:46, Heinrich Schuchardt  wrote:
>
> On 2/13/23 10:42, Masahisa Kojima wrote:
> > Hi Heinrich,
> >
> > On Mon, 13 Feb 2023 at 16:44, Heinrich Schuchardt  
> > wrote:
> >>
> >> On 2/13/23 06:50, Masahisa Kojima wrote:
> >>> On Fri, 10 Feb 2023 at 20:57, Heinrich Schuchardt  
> >>> 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?

Not for the specific message.

At first I planned to add the error message when the variable storage
is not enough to store the efi variable like:

ret = efi_set_variable_int(var_name, &efi_global_variable_guid,
  EFI_VARIABLE_NON_VOLATILE |
  EFI_VARIABLE_BOOTSERVICE_ACCESS |
  EFI_VARIABLE_RUNTIME_ACCESS,
  opt[i].size, opt[i].lo, false);
- if (ret != EFI_SUCCESS)
+ if (ret != EFI_OUT_OF_RESOURCES)
+ efi_print_msg("variable storage is not enough");
+ else if (ret != EFI_XXX)
+ efi_print_msg("another error message");

This will result in an increase of efi_print_msg() calls,
I think it is better to print a status value or text.

Thanks,
Masahisa Kojima

>
>
> > 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 
> > ---
> > 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"},
> > + 

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

2023-02-10 Thread Heinrich Schuchardt

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 
---
  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,