2011/6/16 Cédric VINCENT <cedric.vinc...@st.com>:
> 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
