On 07.04.2011, at 07:56, Ralf Ramsauer wrote: > Here the version with the correct coding style.
Phew - please keep the commit message intact. What we do to apply patches is that we simply directly take your patch into git using "git am". With a patch description like this, if anyone looks at "git blame" or "git log" later, will wonder what the rationale was behind the patch, as the description is basically missing. Also, you need to put a line like this there: Signed-off-by: Ralf Humppa <r...@humppa.name> I would actually simply resend a patch with proper patch description and fixed braces (see next comment), but I can't, because the first patch you sent was already missing your name after the Signed-off-by, basically making the patch a legal grey zone. So please, resend it again with proper patch description, proper Signed-off-by line and the brace thing fixed. Alternatively, I could rewrite the patch myself. But that would obviously rob you off your name in the commit log (due to the missing Signed-off-by), which I bet you wouldn't want :). The fame is all yours after all. > diff --git a/hw/multiboot.c b/hw/multiboot.c > index 0d2bfb4..2380d5e 100644 > --- a/hw/multiboot.c > +++ b/hw/multiboot.c > @@ -267,6 +267,11 @@ int load_multiboot(void *fw_cfg, > /* if a space comes after the module filename, treat everything > after that as parameters */ > target_phys_addr_t c = mb_add_cmdline(&mbs, initrd_filename); > + /* Kill spaces at the beginning of the filename */ Please move your code before the comment above. This way, it's a) confusing to read b) the $0 argument passed to the guest is wrong, as the virtual command line that shows up in the guest multiboot descriptor tables doesn't get the spaces removed. > + while (*initrd_filename == ' ') > + { Please do the brace in the same line as the while. This way it's not following CODING_STYLE. Also, I just realized that you did miss another small part that needs change. A few lines above, there is code to interpret the modules right before that as well: if (initrd_filename) { const char *r = initrd_filename; mbs.mb_buf_size += strlen(r) + 1; mbs.mb_mods_avail = 1; while ((r = strchr(r, ','))) { mbs.mb_mods_avail++; r++; } mbs.mb_buf_size += MB_MOD_SIZE * mbs.mb_mods_avail; } This code would need some change as well, as now the mb_buf_size field is incorrect. It doesn't really hurt, but is bad style to not use the exact amount of memory we need to. > + initrd_filename++; > + } > if ((next_space = strchr(initrd_filename, ' '))) > *next_space = '\0'; > mb_debug("multiboot loading module: %s\n", initrd_filename); I've also CC'ed Adam. He's been the most active person contributing to multiboot recently. Alex