Re: [PATCH v3 37/49] semihosting: Fix docs comment for qemu_semihosting_console_inc

[Peter Maydell]

On Sat, 21 May 2022 at 01:04, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The implementation of qemu_semihosting_console_inc does not
> defer to gdbstub, but only reads from the fifo in console.c.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/semihosting/console.h | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/include/semihosting/console.h b/include/semihosting/console.h
> index 0238f540f4..4f6217bf10 100644
> --- a/include/semihosting/console.h
> +++ b/include/semihosting/console.h
> @@ -41,11 +41,10 @@ void qemu_semihosting_console_outc(CPUArchState *env, 
> target_ulong c);
>   * qemu_semihosting_console_inc:
>   * @env: CPUArchState
>   *
> - * Receive single character from debug console. This may be the remote
> - * gdb session if a softmmu guest is currently being debugged. As this
> - * call may block if no data is available we suspend the CPU and will
> - * re-execute the instruction when data is there. Therefore two
> - * conditions must be met:
> + * Receive single character from debug console.  As this call may block
> + * if no data is available we suspend the CPU and will re-execute the
> + * instruction when data is there. Therefore two conditions must be met:
> + *
>   *   - CPUState is synchronized before calling this function
>   *   - pc is only updated once the character is successfully returned
>   *

Most functions declared here do use the remote gdb connection,
so I think that like qemu_semihosting_log_out() (whose doc comment
includes a sentence "Unlike..." explaining this) we should
explain why this is an exception to that rule ("Unlike...")
rather than just silently not mentioning it. Having 'inc' not
be reading from the same place that 'outc' writes to is rather
unexpected, after all.

thanks
-- PMM