Re: [PATCH 1/2 v2] efi: add a function to convert the status value to string
On Fri, May 03, 2019 at 08:16:42AM +0200, Ard Biesheuvel wrote: > On Fri, 3 May 2019 at 08:15, joeyli wrote: > > > > On Thu, May 02, 2019 at 10:53:31AM +0200, Ard Biesheuvel wrote: > > > On Thu, 2 May 2019 at 06:04, Lee, Chun-Yi wrote: > > > > > > > > This function can be used to convert EFI status value to string > > > > for printing out debug message. Using this function can improve > > > > the readability of log. > > > > > > > > v2. > > > > > > Please move the changelog out of the commit log (move it below the --- > > > further down) > > > > > > > OK! I will moved the changelog out of the commit log. > > > > > > - Changed the wording in subject and description. > > > > - Moved the marco immediately after the status value definitions. > > > > - Turned into a proper function instead of inline. > > > > > > > > > > You missed my point here. A proper function means the function in a .c > > > file, and only the declaration in a .h file. This way, you are still > > > duplicating the literal strings into every object file that references > > > this function. > > > > > > > Sorry for I missunderstood. I will move the function to load_uefi.c. > > > > No, please move it to a file that is shared between all EFI code. I see! I will move the function to the position that it is just behind efi_status_to_str() in drivers/firmware/efi/efi.c. Please let me know if the position is not suitable. Thanks a lot! Joey Lee
Re: [PATCH 1/2 v2] efi: add a function to convert the status value to string
On Fri, 3 May 2019 at 08:15, joeyli wrote: > > On Thu, May 02, 2019 at 10:53:31AM +0200, Ard Biesheuvel wrote: > > On Thu, 2 May 2019 at 06:04, Lee, Chun-Yi wrote: > > > > > > This function can be used to convert EFI status value to string > > > for printing out debug message. Using this function can improve > > > the readability of log. > > > > > > v2. > > > > Please move the changelog out of the commit log (move it below the --- > > further down) > > > > OK! I will moved the changelog out of the commit log. > > > > - Changed the wording in subject and description. > > > - Moved the marco immediately after the status value definitions. > > > - Turned into a proper function instead of inline. > > > > > > > You missed my point here. A proper function means the function in a .c > > file, and only the declaration in a .h file. This way, you are still > > duplicating the literal strings into every object file that references > > this function. > > > > Sorry for I missunderstood. I will move the function to load_uefi.c. > No, please move it to a file that is shared between all EFI code.
Re: [PATCH 1/2 v2] efi: add a function to convert the status value to string
On Thu, May 02, 2019 at 10:53:31AM +0200, Ard Biesheuvel wrote: > On Thu, 2 May 2019 at 06:04, Lee, Chun-Yi wrote: > > > > This function can be used to convert EFI status value to string > > for printing out debug message. Using this function can improve > > the readability of log. > > > > v2. > > Please move the changelog out of the commit log (move it below the --- > further down) > OK! I will moved the changelog out of the commit log. > > - Changed the wording in subject and description. > > - Moved the marco immediately after the status value definitions. > > - Turned into a proper function instead of inline. > > > > You missed my point here. A proper function means the function in a .c > file, and only the declaration in a .h file. This way, you are still > duplicating the literal strings into every object file that references > this function. > Sorry for I missunderstood. I will move the function to load_uefi.c. Thanks a lot! Joey Lee
Re: [PATCH 1/2 v2] efi: add a function to convert the status value to string
On Thu, 2 May 2019 at 06:04, Lee, Chun-Yi wrote:
>
> This function can be used to convert EFI status value to string
> for printing out debug message. Using this function can improve
> the readability of log.
>
> v2.
Please move the changelog out of the commit log (move it below the ---
further down)
> - Changed the wording in subject and description.
> - Moved the marco immediately after the status value definitions.
> - Turned into a proper function instead of inline.
>
You missed my point here. A proper function means the function in a .c
file, and only the declaration in a .h file. This way, you are still
duplicating the literal strings into every object file that references
this function.
> Cc: Ard Biesheuvel
> Cc: Kees Cook
> Cc: Anton Vorontsov
> Cc: Colin Cross
> Cc: Tony Luck
> Signed-off-by: "Lee, Chun-Yi"
> ---
> include/linux/efi.h | 28
> 1 file changed, 28 insertions(+)
>
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 54357a258b35..6f3f89a32eef 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -42,6 +42,34 @@
> #define EFI_ABORTED(21 | (1UL << (BITS_PER_LONG-1)))
> #define EFI_SECURITY_VIOLATION (26 | (1UL << (BITS_PER_LONG-1)))
>
> +#define EFI_STATUS_STR(_status) \
> +case EFI_##_status: \
> + return "EFI_" __stringify(_status);
> +
> +static __attribute__((unused)) char *
> +efi_status_to_str(unsigned long status)
> +{
> + switch (status) {
> + EFI_STATUS_STR(SUCCESS)
> + EFI_STATUS_STR(LOAD_ERROR)
> + EFI_STATUS_STR(INVALID_PARAMETER)
> + EFI_STATUS_STR(UNSUPPORTED)
> + EFI_STATUS_STR(BAD_BUFFER_SIZE)
> + EFI_STATUS_STR(BUFFER_TOO_SMALL)
> + EFI_STATUS_STR(NOT_READY)
> + EFI_STATUS_STR(DEVICE_ERROR)
> + EFI_STATUS_STR(WRITE_PROTECTED)
> + EFI_STATUS_STR(OUT_OF_RESOURCES)
> + EFI_STATUS_STR(NOT_FOUND)
> + EFI_STATUS_STR(ABORTED)
> + EFI_STATUS_STR(SECURITY_VIOLATION)
> + default:
> + pr_warn("Unknown efi status: 0x%lx", status);
> + }
> +
> + return "Unknown efi status";
> +}
> +
> typedef unsigned long efi_status_t;
> typedef u8 efi_bool_t;
> typedef u16 efi_char16_t; /* UNICODE character */
> --
> 2.16.4
>
