Can you not just mask of the error bit, and index into the table (after a bounds check) as opposed to doing this looping search?
Also I think an strerror type interface would be nicer to callers (maybe in addition) as opposed to this perror stuff, that way they can do their own grub_error, or other thing as they wish. On Thu, Jun 26, 2025 at 10:25 AM Khalid Ali <khaliidca...@gmail.com> wrote: > > This RFC patch introduces a UEFI error message printer that translates > UEFI status codes from hexadecimal values into human-readable strings. > > The problem this rfc solves is, undescriptive error messages in Grub (on > failure what > get printed). Specifically the documented firmware spec which UEFI. In UEFI > specific code > i noticed error messages like "unknown error", "cannot load image" and so on. > Unfortunately > changing error message one by one is expensive and bad idea, all privious > attempt resulted > alot of code duplication. So this idea of centralized error handling for UEFI > specific was the > easiest and cleanest way the problem could be solved. > > The idea is inspired by the GNU C Library's "perror()" function, which > translates > "errno" values into descriptive error messages. Similarly, this > implementation eliminates > the need for conditional statements when printing UEFI error messages. In > short, it's a "perror"-like > mechanism tailored for UEFI status codes in GRUB. > > The function takes three parameter: > - "grub_err_t err": if the caller desires to set errno. Caller can safely > return errno if it wants to return status code. > - "const char *s": a null-terminated string that further > describes the error context, like the reason of failure. > - "EFI_STATUS status": the UEFI status code returned by a call to UEFI > service, to translate to string. > > Additionally, if this patch is accepted, it maybe better if we propogate > this function throughout UEFI subsystem. That would mean replacing some > grub_error() calls to this function as it is the one who is calling it > internally, putting on UEFI service calls in case of failure to check and > print > status codes. Then the caller can safely return "grub_errno" if there > were a failure. The benefit would be clearer, and more descriptive error > messages, > instead of raw hex codes and reduction of the amount of code needed for > printing error messages like conditional statements. This will improve user > experience, > debugging clarity and code readability. > > Looking forward for your feedback and what community think of this idea > and the code. Additionally i left something i felt worth dicussing with > the commuinty, which is if translation is ideal on this context. > > Best regard > khaalid > > Signed-off-by: Khalid Ali <khaliidca...@gmail.com> > --- > grub-core/kern/efi/err.c | 81 ++++++++++++++++++++++++++++++++++++++++ > include/grub/efi/api.h | 6 +++ > include/grub/efi/efi.h | 3 ++ > 3 files changed, 90 insertions(+) > create mode 100644 grub-core/kern/efi/err.c > > diff --git a/grub-core/kern/efi/err.c b/grub-core/kern/efi/err.c > new file mode 100644 > index 000000000..b870b26ba > --- /dev/null > +++ b/grub-core/kern/efi/err.c > @@ -0,0 +1,81 @@ > +/* err.c - uefi specific error handling routine */ > +/* > + * GRUB -- GRand Unified Bootloader > + * Copyright (C) 2025 Free Software Foundation, Inc. > + * > + * GRUB is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * GRUB is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <grub/err.h> > +#include <grub/misc.h> > +#include <grub/efi/api.h> > +#include <grub/efi/efi.h> > + > +struct grub_efi_err > +{ > + grub_efi_status_t status; > + const char *error_msg; > +}; > + > +/* EFI error table: keep entries sorted alphabetically. */ > +static const struct grub_efi_err grub_efi_err_table[] = > +{ > + {GRUB_EFI_ABORTED, "aborted"}, > + {GRUB_EFI_ACCESS_DENIED, "access denied"}, > + {GRUB_EFI_ALREADY_STARTED, "already started"}, > + {GRUB_EFI_BAD_BUFFER_SIZE, "bad buffer size"}, > + {GRUB_EFI_BUFFER_TOO_SMALL, "buffer too small"}, > + {GRUB_EFI_CRC_ERROR, "crc error"}, > + {GRUB_EFI_DEVICE_ERROR, "device error"}, > + {GRUB_EFI_END_OF_FILE, "end of file"}, > + {GRUB_EFI_END_OF_MEDIA, "end of media"}, > + {GRUB_EFI_HTTP_ERROR, "http error"}, > + {GRUB_EFI_ICMP_ERROR, "icmp error"}, > + {GRUB_EFI_INCOMPATIBLE_VERSION, "incompatible version"}, > + {GRUB_EFI_INVALID_PARAMETER, "invalid parameter"}, > + {GRUB_EFI_IP_ADDRESS_CONFLICT, "ip address conflict"}, > + {GRUB_EFI_LOAD_ERROR, "load error"}, > + {GRUB_EFI_MEDIA_CHANGED, "media changed"}, > + {GRUB_EFI_NO_MAPPING, "no mapping"}, > + {GRUB_EFI_NO_MEDIA, "no media"}, > + {GRUB_EFI_NO_RESPONSE, "no response"}, > + {GRUB_EFI_NOT_FOUND, "not found"}, > + {GRUB_EFI_NOT_READY, "not ready"}, > + {GRUB_EFI_NOT_STARTED, "not started"}, > + {GRUB_EFI_OUT_OF_RESOURCES, "out of resources"}, > + {GRUB_EFI_PROTOCOL_ERROR, "protocol error"}, > + {GRUB_EFI_SECURITY_VIOLATION, "security violation"}, > + {GRUB_EFI_SUCCESS, "success"}, > + {GRUB_EFI_TFTP_ERROR, "tftp error"}, > + {GRUB_EFI_TIMEOUT, "timed out"}, > + {GRUB_EFI_UNSPECIFIED_TIMEZONE, "unspecified timezone"}, > + {GRUB_EFI_UNSUPPORTED, "unsupported"}, > + {GRUB_EFI_VOLUME_CORRUPTED, "volume corrupted"}, > + {GRUB_EFI_VOLUME_FULL, "volume full"}, > + {GRUB_EFI_WRITE_PROTECTED, "write protected"} > +}; > + > +void > +grub_efi_perror(grub_err_t err, const char *s, grub_efi_status_t status) > +{ > + for (grub_size_t i = 0; i < ARRAY_SIZE(grub_efi_err_table); i++) > + { > + if (grub_efi_err_table[i].status == status) > + { > + grub_error (err, "%s : %s", s, grub_efi_err_table[i].error_msg); > + return; /* we got what we needed, that is enough*/ > + } > + } > + grub_error (err, "%s : unknown error", s); > +} > \ No newline at end of file > diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h > index b686e8afe..f74bc0287 100644 > --- a/include/grub/efi/api.h > +++ b/include/grub/efi/api.h > @@ -632,6 +632,12 @@ typedef grub_efi_uintn_t grub_efi_status_t; > #define GRUB_EFI_INCOMPATIBLE_VERSION GRUB_EFI_ERROR_CODE (25) > #define GRUB_EFI_SECURITY_VIOLATION GRUB_EFI_ERROR_CODE (26) > #define GRUB_EFI_CRC_ERROR GRUB_EFI_ERROR_CODE (27) > +#define GRUB_EFI_END_OF_MEDIA GRUB_EFI_ERROR_CODE (28) > +#define GRUB_EFI_END_OF_FILE GRUB_EFI_ERROR_CODE (31) > +#define GRUB_EFI_INVALID_LANGUAGE GRUB_EFI_ERROR_CODE (32) > +#define GRUB_EFI_COMPROMISED_DATA GRUB_EFI_ERROR_CODE (33) > +#define GRUB_EFI_IP_ADDRESS_CONFLICT GRUB_EFI_ERROR_CODE (34) > +#define GRUB_EFI_HTTP_ERROR GRUB_EFI_ERROR_CODE (35) > > #define GRUB_EFI_WARN_UNKNOWN_GLYPH GRUB_EFI_WARNING_CODE (1) > #define GRUB_EFI_WARN_DELETE_FAILURE GRUB_EFI_WARNING_CODE (2) > diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h > index a5cd99e5a..8e91eaad4 100644 > --- a/include/grub/efi/efi.h > +++ b/include/grub/efi/efi.h > @@ -122,6 +122,9 @@ extern void (*EXPORT_VAR(grub_efi_net_config)) > (grub_efi_handle_t hnd, > void * > EXPORT_FUNC (grub_efi_find_configuration_table) (const grub_guid_t > *target_guid); > > +void > +EXPORT_FUNC (grub_efi_perror) (grub_err_t err, const char *s, > grub_efi_status_t status); > + > #if defined(__arm__) || defined(__aarch64__) || defined(__riscv) || > defined(__loongarch__) > void *EXPORT_FUNC(grub_efi_get_firmware_fdt)(void); > grub_err_t EXPORT_FUNC(grub_efi_get_ram_base)(grub_addr_t *); > -- > 2.49.0 > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
