Re: [Qemu-devel] [PATCH] arm-semi: Provide access to CLI arguments passed through the "-append" option

[Peter Maydell]

2011/6/16 Cédric VINCENT <address@hidden>:
> This patch basically adapts the new semi-hosting command-line support
> -- introduced by Wolfgang Schildbach in the commit 2e8785ac -- for use
> in system-mode.

Generally looks OK. Some nits:

> -        /* Build a commandline from the original argv.  */
> +        /* Build a command-line from the original argv.
> +         *

If you're going to expand this into a multiline comment I'd prefer
it to be inside the brace rather than outside.

> +            if (!input_size || output_size > input_size) {
> +                 /* Not enough space to store command-line arguments.  */
> +                return -1;

You can drop the check for !input_size here, because you've eliminated
the case where output_size == 0, and so a zero input_size will be
caught by the other half of the conditional.

> +            /* Separate arguments by white spaces.  */
> +            for (i = 0; i < output_size; i++) {
> +                if (output_buffer[i] == 0) {
> +                    output_buffer[i] = ' ';
> +                }
> +            }

This will turn the final trailing NUL into a space -- should
be "i < output_size - 1".

> +        out:
> +#endif
> +            /* Unlock the buffer on the ARM side.  */
> +            unlock_user(output_buffer, ARG(0), output_size);
>
> -            /* Return success if we could return a commandline.  */
> -            return (arm_cmdline_buffer && host_cmdline_buffer) ? 0 : -1;
> +            return status;
>         }
> -#else
> -        return -1;
> -#endif
> +        /* Never reached.  */

This is kind of obvious so I think the comment is unnecessary.

-- PMM