Re: [PATCH v3 2/3] i386: Add support for loading from android bootimg

[Andrei Borzenkov]

13.02.2016 00:16, Vladimir 'φ-coder/phcoder' Serbinenko пишет:
> On 12.02.2016 20:34, Shea Levy wrote:
>> > OK. Do you have any thoughts on how best to extract the "load the kernel
>> > and set the command line to a specific string" functionality out of the
>> > linux command, then?
>> > 
> At first I wanted to write a short skeleton patch to show what I meant
> but once skeleton is done, there is little actual code involved. Please
> try attached patch

Yes, exactly what I meant. Nitpicks below.

> android.diff
> 
> 
> diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
> index fddcc46..d3870ae 100644
> --- a/grub-core/loader/i386/linux.c
> +++ b/grub-core/loader/i386/linux.c
> @@ -35,6 +35,7 @@
>  #include <grub/i18n.h>
>  #include <grub/lib/cmdline.h>
>  #include <grub/linux.h>
> +#include <grub/android.h>
>  
>  GRUB_MOD_LICENSE ("GPLv3+");
>  
> @@ -673,11 +674,11 @@ grub_linux_unload (void)
>    return GRUB_ERR_NONE;
>  }
>  
> +
>  static grub_err_t
> -grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
> -             int argc, char *argv[])
> +linux_load (grub_file_t file, grub_off_t fileoffset, grub_off_t filesize,
> +         const char *extra_cmdline, int argc, char *argv[])
>  {
> -  grub_file_t file = 0;
>    struct linux_kernel_header lh;
>    grub_uint8_t setup_sects;
>    grub_size_t real_size, prot_size, prot_file_size;
> @@ -689,15 +690,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ 
> ((unused)),
>  
>    grub_dl_ref (my_mod);
>  
> -  if (argc == 0)
> -    {
> -      grub_error (GRUB_ERR_BAD_ARGUMENT, N_("filename expected"));
> -      goto fail;
> -    }
> -
> -  file = grub_file_open (argv[0]);
> -  if (! file)
> -    goto fail;
> +  grub_file_seek (file, fileoffset);
>  
>    if (grub_file_read (file, &lh, sizeof (lh)) != sizeof (lh))
>      {
> @@ -757,7 +750,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ 
> ((unused)),
>      setup_sects = GRUB_LINUX_DEFAULT_SETUP_SECTS;
>  
>    real_size = setup_sects << GRUB_DISK_SECTOR_BITS;
> -  prot_file_size = grub_file_size (file) - real_size - GRUB_DISK_SECTOR_SIZE;
> +  prot_file_size = filesize - real_size - GRUB_DISK_SECTOR_SIZE;
>  
>    if (grub_le_to_cpu16 (lh.version) >= 0x205
>        && lh.kernel_alignment != 0
> @@ -871,7 +864,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ 
> ((unused)),
>  
>    /* The other parameters are filled when booting.  */
>  
> -  grub_file_seek (file, real_size + GRUB_DISK_SECTOR_SIZE);
> +  grub_file_seek (file, fileoffset + real_size + GRUB_DISK_SECTOR_SIZE);
>  
>    grub_dprintf ("linux", "bzImage, setup=0x%x, size=0x%x\n",
>               (unsigned) real_size, (unsigned) prot_size);
> @@ -1008,12 +1001,32 @@ grub_cmd_linux (grub_command_t cmd __attribute__ 
> ((unused)),
>    linux_cmdline = grub_zalloc (maximal_cmdline_size + 1);
>    if (!linux_cmdline)
>      goto fail;
> -  grub_memcpy (linux_cmdline, LINUX_IMAGE, sizeof (LINUX_IMAGE));
> -  grub_create_loader_cmdline (argc, argv,
> -                           linux_cmdline
> -                           + sizeof (LINUX_IMAGE) - 1,
> -                           maximal_cmdline_size
> -                           - (sizeof (LINUX_IMAGE) - 1));
> +  char *cmdline_ptr = linux_cmdline;
> +  char *cmdline_ptr_end = linux_cmdline + maximal_cmdline_size;
> +  /* Construct BOOT_IMAGE=/boot/... */
> +  cmdline_ptr = grub_stpcpy (cmdline_ptr, LINUX_IMAGE);
> +  grub_create_loader_cmdline (1, argv, cmdline_ptr,

What if argc is zero? We checked this before, but check is removed. This
became library function now, it should check input for sanity.
...

> diff --git a/include/grub/misc.h b/include/grub/misc.h
> index 2a9f87c..ef9c9b6 100644
> --- a/include/grub/misc.h
> +++ b/include/grub/misc.h
> @@ -64,6 +64,17 @@ grub_stpcpy (char *dest, const char *src)
>    return d - 1;
>  }
>  
> +static inline char *
> +grub_stpncpy (char *dest, const char *src, int c)

"Official" prototype has size_t here.

signature.asc
Description: OpenPGP digital signature