hw: improve multiboot module loading

Submitted by ralf@humppa.name on April 7, 2011, 12:19 a.m.

Details

Message ID 312edea8efadfde52c4c6d267c924cf2.squirrel@humppa.name
State New
Headers show

Commit Message

ralf@humppa.name April 7, 2011, 12:19 a.m.
Multiboot modules couldn't be loaded when there are spaces between the
filename and ','. Those spaces can simply be killed.

Signed-off-by:
---

Comments

Alexander Graf April 7, 2011, 12:31 a.m.
On 07.04.2011, at 02:19, ralf@humppa.name wrote:

> Multiboot modules couldn't be loaded when there are spaces between the
> filename and ','. Those spaces can simply be killed.
> 
> Signed-off-by:
> ---
> diff --git a/hw/multiboot.c b/hw/multiboot.c
> index 0d2bfb4..27eb159 100644
> --- a/hw/multiboot.c
> +++ b/hw/multiboot.c
> @@ -267,6 +267,9 @@ 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 */
> +            while( *initrd_filename == ' ' )
> +              initrd_filename++;

Please make sure to follow the coding style. There shouldn't be whitespace between the (), but between while and ( and you need some extra braces :):

while (*initrd_filename == ' ') {
    initrd_filename++;
}

Otherwise, the patch looks good! Please resend a corrected version, so that it can be easily applied.


Alex
Brad Hards April 7, 2011, 2 a.m.
On Thu, 7 Apr 2011 10:19:01 am ralf@humppa.name wrote:
> Signed-off-by:
Looks like something may be wrong with your git config

Brad
Stefan Hajnoczi April 7, 2011, 8:43 a.m.
On Thu, Apr 07, 2011 at 12:19:01AM -0000, ralf@humppa.name wrote:
> @@ -267,6 +267,9 @@ 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 */
> +            while( *initrd_filename == ' ' )
> +              initrd_filename++;

If we want to do this, shouldn't it be done before adding to the
multiboot command-line?  Otherwise the multiboot image also has to strip
leading spaces.

Stefan
ralf@humppa.name April 7, 2011, 10:18 a.m.
Leading spaces mustn't be stripped! The commandline could look like:
qemu -kernel kern -initrd "1.mod arg=foo arg2=bar, 2.mod arg=asdf"
The problem is, that the string
"1.mod arg=foo arg2=bar, 2.mod arg=asdf"
is divided at the ','. So we have two string
"1.mod arg=foo arg2=bar" and
" 2.mod arg=asdf"
 ^ This space causes the error "Failed to get image size"
So just the spaces at the beginning of the file have to be stripped.
Spaces at the end could be important for the commandline. E.g.
-initrd "1.mod arg=argwithmanyspaces    , 2.mod"

--
Ralf

> If we want to do this, shouldn't it be done before adding to the
> multiboot command-line?  Otherwise the multiboot image also has to strip
> leading spaces.
>
> Stefan
>

Patch hide | download patch | download mbox

diff --git a/hw/multiboot.c b/hw/multiboot.c
index 0d2bfb4..27eb159 100644
--- a/hw/multiboot.c
+++ b/hw/multiboot.c
@@ -267,6 +267,9 @@  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 */
+            while( *initrd_filename == ' ' )
+              initrd_filename++;
             if ((next_space = strchr(initrd_filename, ' ')))
                 *next_space = '\0';
             mb_debug("multiboot loading module: %s\n", initrd_filename);