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