Le dim. 11 mai 2025, 17:36, khaalid cali <khaliidca...@gmail.com> a écrit :
> From: khaalid <khaliidc...@gmail.com>
>
> I followed the suggestion and implemented the option parsing using extcmd.
> However, I feel this command is not ideal to have any arguments. Similar
> commands like
> lsefi, lsefimmap, and lsefisystab don’t take arguments—they simply dump
> data.
> Since this command also just dumps variables and doesn’t do anything more,
> adding options might introduce inconsistency.
>
> Returning grub_err_t for `dump_variable_data` isn't ideal neither for
> two reasons:
> - First the only place the function fail is if
> `grub_efi_get_variable_with_attributes`
> fail; this function returns EFI_STATUS mostly, so even if it fails and
> sets errno then
> `grub_error` already overrided it so we get GRUB_ERR_IO always.
> - Second `dump_variable_data` is expected to fail because not every
> variable can neccessarily retrieved, some will fail due security
> restrictions. So it is better
> if we skip failed variables and move on to the next one.
>
If you want to ignore error, you need to reset grub_errno to GRUB_ERR_NONE,
not call grub_error. Then you can keep it void
>
> +static void
> +dump_variable_data(const char *variable_name, grub_guid_t *variable_guid)
> +{
> + grub_efi_status_t status;
> + grub_efi_uint32_t attributes;
> + grub_size_t data_size;
> + void *data;
> +
> + status = grub_efi_get_variable_with_attributes(variable_name,
> variable_guid,
> + &data_size, &data,
> &attributes);
> + if (status != GRUB_EFI_SUCCESS)
> + {
> + grub_error(GRUB_ERR_IO, "failed to retrieve variable data 0x%"
> PRIxGRUB_EFI_UINTN_T, status);
>
Already said.
+ return;
> + }
> +
> + grub_printf(_("Attributes:\n"));
>
Actually grub_puts_ is preferred when they're are no arguments. Sorry, I
forgot this.
> + variable_name = grub_realloc(variable_name,
> variable_name_size * sizeof(grub_efi_char16_t));
> + if (variable_name == NULL)
>
Please have a look at how to handle errors with realloc. You need to keep a
copy of old pointer in case you have to free it. Some places in our
codebase have similar memory leaks but it's not a reason to introduce more.
> + return grub_errno;
> + }
>
>
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
