Alexandre Boeglin <[EMAIL PROTECTED]> writes: > On Sun, 4 Nov 2007 01:21:24 +0100, Alexandre Boeglin <[EMAIL PROTECTED]> > wrote: >> Le Sun, Nov 04, 2007 at 01:04:24AM +0100, Alexandre Boeglin a écrit : >>> Sorry, there was a mistake in the previous one ... >> Oops, and line 91 of this one should be > > Hi, here is a "more correct" version of this patch: it also frees the > memory allocated to the options string when appropriate.
I should've started with this patch ;-) I can better do a full review again for clarity :-) > Index: include/grub/efi/chainloader.h > =================================================================== > RCS file: /sources/grub/grub2/include/grub/efi/chainloader.h,v > retrieving revision 1.2 > diff -u -r1.2 chainloader.h > --- include/grub/efi/chainloader.h 21 Jul 2007 23:32:22 -0000 1.2 > +++ include/grub/efi/chainloader.h 5 Nov 2007 22:58:51 -0000 > @@ -19,6 +19,6 @@ > #ifndef GRUB_EFI_CHAINLOADER_HEADER > #define GRUB_EFI_CHAINLOADER_HEADER 1 > > -void grub_chainloader_cmd (const char *filename); > +void grub_chainloader_cmd (int argc, char *argv[]); > > #endif /* ! GRUB_EFI_CHAINLOADER_HEADER */ > Index: loader/efi/chainloader.c > =================================================================== > RCS file: /sources/grub/grub2/loader/efi/chainloader.c,v > retrieving revision 1.2 > diff -u -r1.2 chainloader.c > --- loader/efi/chainloader.c 21 Jul 2007 23:32:28 -0000 1.2 > +++ loader/efi/chainloader.c 5 Nov 2007 22:58:58 -0000 > @@ -17,8 +17,6 @@ > * along with GRUB. If not, see <http://www.gnu.org/licenses/>. > */ > > -/* TODO: support load options. */ > - > #include <grub/loader.h> > #include <grub/file.h> > #include <grub/err.h> > @@ -38,6 +36,8 @@ > > static grub_efi_physical_address_t address; > static grub_efi_uintn_t pages; > +static grub_efi_char16_t *options; > +static grub_efi_uintn_t options_len; > static grub_efi_device_path_t *file_path; > static grub_efi_handle_t image_handle; > > @@ -49,6 +49,8 @@ > b = grub_efi_system_table->boot_services; > b->unload_image (image_handle); > b->free_pages (address, pages); > + if (options) > + b->free_pool (options); > grub_free (file_path); > > grub_dl_unref (my_mod); > @@ -175,7 +177,7 @@ > } > > void > -grub_chainloader_cmd (const char *filename) > +grub_chainloader_cmd (int argc, char *argv[]) > { > grub_file_t file = 0; > grub_ssize_t size; > @@ -185,16 +187,47 @@ > grub_device_t dev = 0; > grub_efi_device_path_t *dp = 0; > grub_efi_loaded_image_t *loaded_image; > + char *filename = argv[0]; > > grub_dl_ref (my_mod); > > /* Initialize some global variables. */ > address = 0; > + options = 0; > + options_len = 0; > image_handle = 0; > file_path = 0; > > b = grub_efi_system_table->boot_services; > > + if (argc == 0) > + { > + grub_error (GRUB_ERR_BAD_ARGUMENT, "no file specified"); > + goto fail; > + } The indentation is not right. > + /* copy options */ /* Copy options. */ > + if (argc > 1) > + { The indentation is not right. > + grub_efi_char16_t *p; > + grub_efi_int16_t i; > + grub_efi_uint16_t j; > + for (i = 1; i < argc; i++) > + options_len += (grub_strlen (argv[i]) + 1) * sizeof (*options); > + options_len += sizeof (*options); /* \0 */ > + > + status = b->allocate_pool (GRUB_EFI_LOADER_DATA, options_len, (void *) > &options); > + if (status != GRUB_EFI_SUCCESS) > + goto fail; > + p = options; Please use grub_malloc here, I think it should work for what you are doing, right? > + for (i = 1; i < argc; i++){ > + *p++ = ' '; > + for (j = 0; j < grub_strlen (argv[i]) + 1; j++) > + p[j] = (grub_efi_char16_t) argv[i][j]; > + } > + } > + > file = grub_file_open (filename); > if (! file) > goto fail; > @@ -268,6 +301,10 @@ > } > loaded_image->device_handle = dev_handle; > > + if (options_len) > + loaded_image->load_options = options; > + loaded_image->load_options_size = options_len; The second loaded_image... line seems to be incorrectly indented. > grub_file_close (file); > grub_loader_set (grub_chainloader_boot, grub_chainloader_unload, 0); > return; > @@ -286,16 +323,16 @@ > if (address) > b->free_pages (address, pages); > > + if (options) > + b->free_pool (options); > + > grub_dl_unref (my_mod); > } > > static void > grub_rescue_cmd_chainloader (int argc, char *argv[]) > { > - if (argc == 0) > - grub_error (GRUB_ERR_BAD_ARGUMENT, "no file specified"); > - else > - grub_chainloader_cmd (argv[0]); > + grub_chainloader_cmd (argc, argv); > } > > static const char loader_name[] = "chainloader"; > Index: loader/efi/chainloader_normal.c > =================================================================== > RCS file: /sources/grub/grub2/loader/efi/chainloader_normal.c,v > retrieving revision 1.2 > diff -u -r1.2 chainloader_normal.c > --- loader/efi/chainloader_normal.c 21 Jul 2007 23:32:28 -0000 1.2 > +++ loader/efi/chainloader_normal.c 5 Nov 2007 22:58:59 -0000 > @@ -26,10 +26,7 @@ > chainloader_command (struct grub_arg_list *state __attribute__ ((unused)), > int argc, char **args) > { > - if (argc == 0) > - grub_error (GRUB_ERR_BAD_ARGUMENT, "no file specified"); > - else > - grub_chainloader_cmd (args[0]); > + grub_chainloader_cmd (argc, args); > return grub_errno; > } _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
