On Tue, Jun 24, 2025 at 4:48 PM Daniel Kiper <dki...@net-space.pl> wrote: > > 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. >
Added a commit explaining why this was changed. > > 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; > Done. > > + 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. > I split the various changes, see v3 series. > Daniel Frediano _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel