On Sat, Aug 01, 2009 at 03:23:04PM +0200, Vladimir 'phcoder' Serbinenko wrote:
> According to multiboot specification "It should be possible to write
> compliant boot loaders that load the OS image from a variety of
> sources, including floppy disk, hard disk, and across a network. "
> It implicitly says that kernel shouldn't care about filename used by
> bootloader. About commandline it's only said:
> "If bit 2 of the `flags' longword is set, the `cmdline' field is
> valid, and contains the physical address of the command line to be
> passed to the kernel. The command line is a normal C-style
> zero-terminated string"
> Currently grub however passes filename as first argument of command line.
> Felix Zielcke did a series of tests and it revealed that OSes
> subdivide into two categories:
> 1) Ones that don't care whether this argument is omitted altogether or
> not (e.g. the don't ignore second argument even if filename is
> omitted)
> 2) Solaris (for kernel name) and AROS (for module name) both assume
> that filename is the same under grub and OS. This assumption isn't
> necessarily true. These OSes are broken one way or another. If we omit
> the first argument user can workaround the problem by supplying
> kernel/module OS image name as first argument of multiboot/module
> command line
> E.g.
> multiboot /RPOOL/opensolaris/@/platform/i86pc/kernel /platform/i86pc/kernel
I agree with this. But please wait a few days to give everyone a chance
to read it.
Regarding the patch:
> /* Figure out cmdline length. */
> - for (i = 0, cmdline_length = 0; i < argc; i++)
> + for (i = 1, cmdline_length = 0; i < argc; i++)
> cmdline_length += grub_strlen (argv[i]) + 1;
>
> + if (cmdline_length == 0)
> + cmdline_length = 1;
> +
>
> boot_loader_name_length = sizeof(PACKAGE_STRING);
>
> #define cmdline_addr(x) ((void *) ((x) + code_size))
> @@ -351,14 +354,16 @@ grub_multiboot (int argc, char *argv[])
> if (! cmdline)
> goto fail;
>
> - for (i = 0; i < argc; i++)
> + for (i = 1; i < argc; i++)
> {
> p = grub_stpcpy (p, argv[i]);
> *(p++) = ' ';
> }
>
> /* Remove the space after the last word. */
> - *(--p) = '\0';
> + if (p != cmdline)
> + p--;
> + *p = 0;
>
> mbi->flags |= MULTIBOOT_INFO_CMDLINE;
> mbi->cmdline = (grub_uint32_t) cmdline_addr (grub_multiboot_payload_dest);
> @@ -422,9 +427,12 @@ grub_module (int argc, char *argv[])
> goto fail;
> }
>
> - for (i = 0; i < argc; i++)
> + for (i = 1; i < argc; i++)
> len += grub_strlen (argv[i]) + 1;
>
> + if (len == 0)
> + len = 1;
> +
> cmdline = p = grub_malloc (len);
> if (! cmdline)
> goto fail;
> @@ -436,7 +444,9 @@ grub_module (int argc, char *argv[])
> }
>
> /* Remove the space after the last word. */
> - *(--p) = '\0';
> + if (p != cmdline)
> + p--;
> + *p = '\0';
There's a much simpler way to address this. Just add something like:
cmdline_argv = argv + 1;
cmdline_argc = argc - 1;
at the beginning, and then use cmdline_argv and cmdline_argc instead of
correcting the off-by-one every time.
--
Robert Millan
The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
how) you may access your data; but nobody's threatening your freedom: we
still allow you to remove your data and not access it at all."
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel
