25.02.2017 02:22, Vladimir 'phcoder' Serbinenko пишет: > On Sun, Feb 12, 2017, 11:52 Vladimir 'phcoder' Serbinenko <phco...@gmail.com> > wrote: > >> >> >> On Sun, 12 Feb 2017, 08:06 Andrei Borzenkov <arvidj...@gmail.com> wrote: >> >> 12.02.2017 00:07, Vladimir 'phcoder' Serbinenko пишет: >> ... >>>> >>> For multiboot module old grub was passing module file name on module >>> command line, so legacy_configfile should replicate this behaviour. But >> in >>> Linux case it shouldn't have ended up loading twice. We need additional >>> code in legacy_initrd. Please try the attached patch >>> >> ... >>> >>> >>> diff --git a/grub-core/commands/legacycfg.c >> b/grub-core/commands/legacycfg.c >>> index dd9d9f1..24546bf 100644 >>> --- a/grub-core/commands/legacycfg.c >>> +++ b/grub-core/commands/legacycfg.c >>> @@ -517,7 +517,7 @@ grub_cmd_legacy_initrd (struct grub_command *mycmd >> __attribute__ ((unused)), >>> #endif >>> ); >>> >>> - return cmd->func (cmd, argc, args); >>> + return cmd->func (cmd, argc ? argc - 1 : 0, args + 1); >> >> If the goal is to be compatible with legacy grub, it is not compatible >> (nor was) - legacy grub "initrd" ignored all extra arguments to "initrd" >> command while we attempt to load them as additional initrd components. >> >> Nor did legacy grub attempt to quote command line for multiboot kernel >> or modules. >> >> Agreed on both. That's why I said that previous patch is wrong and I'm >> working on new one >> > For first point, see attached patch. > Second point isn't a problem as legacycfg will already split along spaces > and hence quoting code will not be triggered. >
It will. while (*c) { if (*c == '\\' || *c == '\'' || *c == '"') *buf++ = '\\'; *buf++ = *c; c++; } In particular, this is a problem with Linux which does not interpret these quotes at all (it only supports " .... " without any way to escape nested `"'). That's something for post 2.02 now, but I still would like to know what was the reason for this code in the first place. > > diff --git a/grub-core/commands/legacycfg.c b/grub-core/commands/legacycfg.c > index dd9d9f1..b32f3c7 100644 > --- a/grub-core/commands/legacycfg.c > +++ b/grub-core/commands/legacycfg.c > @@ -517,7 +517,7 @@ grub_cmd_legacy_initrd (struct grub_command *mycmd > __attribute__ ((unused)), > #endif > ); > > - return cmd->func (cmd, argc, args); > + return cmd->func (cmd, argc ? 1 : 0, args); > } > if (kernel_type == MULTIBOOT) > { > Looks OK as stop gap. @Andy, could you test it? I still do not understand why initrd is parsed as TYPE_FILE_NO_CONSUME, nor why we handle multiboot kernel in legacy_initrd in the first place. Legacy grub only supported Linux kernel with initrd command. switch (kernel_type) { case KERNEL_TYPE_LINUX: case KERNEL_TYPE_BIG_LINUX: if (! load_initrd (arg)) return 1; break; default: errnum = ERR_NEED_LX_KERNEL; return 1; } Although this probably does not really matter now. _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel