On Tue, Jun 24, 2025 at 09:01:14AM +0100, Frediano Ziglio via Grub-devel wrote: > If a simple string for arguments are passed it should be NUL > terminated. This is true for other code but not for "linux" > command. > Signed-off-by: Frediano Ziglio <frediano.zig...@cloud.com> > --- > Changes since v1 > - remove useless assignment. > --- > grub-core/loader/efi/linux.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/grub-core/loader/efi/linux.c b/grub-core/loader/efi/linux.c > index 78ea07ca8..0c14f6d8f 100644 > --- a/grub-core/loader/efi/linux.c > +++ b/grub-core/loader/efi/linux.c > @@ -190,7 +190,7 @@ grub_arch_efi_linux_boot_image (grub_addr_t addr, > grub_size_t size, char *args) > grub_efi_boot_services_t *b; > grub_efi_status_t status; > grub_efi_loaded_image_t *loaded_image; > - int len; > + grub_size_t len;
Is it really need? If yes this change should be explained in the commit message. > mempath = grub_malloc (2 * sizeof (grub_efi_memory_mapped_device_path_t)); > if (!mempath) > @@ -223,16 +223,17 @@ grub_arch_efi_linux_boot_image (grub_addr_t addr, > grub_size_t size, char *args) > grub_error (GRUB_ERR_BAD_FIRMWARE, "missing loaded_image proto"); > goto unload; > } > - loaded_image->load_options_size = len = > - (grub_strlen (args) + 1) * sizeof (grub_efi_char16_t); > + len = (grub_strlen (args) + 1) * sizeof (grub_efi_char16_t); > loaded_image->load_options = > grub_efi_allocate_any_pages (GRUB_EFI_BYTES_TO_PAGES (len)); > if (!loaded_image->load_options) > return grub_errno; > > - loaded_image->load_options_size = > - 2 * grub_utf8_to_utf16 (loaded_image->load_options, len, > - (grub_uint8_t *) args, len, NULL); > + len = grub_utf8_to_utf16 (loaded_image->load_options, len, > + (grub_uint8_t *) args, (grub_size_t) -1, NULL); > + /* NUL terminate */ > + ((grub_efi_char16_t*)loaded_image->load_options)[len++] = 0; Incorrectly formatted cast... ((grub_efi_char16_t *) loaded_image->load_options)[len++] = 0; > + loaded_image->load_options_size = len * sizeof (grub_efi_char16_t); In general I am not happy with the code shuffling which you are doing in this patch. You have two options here: you should explain in the commit message why the shuffling is needed or split patch into two and do required shuffling, with proper commit message, in the first one and add NUL terminator in the second. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
