Re: [PULL v2 11/14] semihosting: add qemu_semihosting_console_inc for SYS_READC

[Peter Maydell]

On Thu, 9 Jan 2020 at 14:19, Alex Bennée <address@hidden> wrote:
>
> From: Keith Packard <address@hidden>
>
> Provides a blocking call to read a character from the console using
> semihosting.chardev, if specified. This takes some careful command
> line options to use stdio successfully as the serial ports, monitor
> and semihost all want to use stdio. Here's a sample set of command
> line options which share stdio between semihost, monitor and serial
> ports:

Hi; Coverity has some complaints about this code, and
specifically the use of getchar():

> +/*
> + * For linux-user we can safely block. However as we want to return as
> + * soon as a character is read we need to tweak the termio to disable
> + * line buffering. We restore the old mode afterwards in case the
> + * program is expecting more normal behaviour. This is slow but
> + * nothing using semihosting console reading is expecting to be fast.
> + */
> +target_ulong qemu_semihosting_console_inc(CPUArchState *env)
> +{
> +    uint8_t c;
> +    struct termios old_tio, new_tio;
> +
> +    /* Disable line-buffering and echo */
> +    tcgetattr(STDIN_FILENO, &old_tio);
> +    new_tio = old_tio;
> +    new_tio.c_lflag &= (~ICANON & ~ECHO);
> +    tcsetattr(STDIN_FILENO, TCSANOW, &new_tio);
> +
> +    c = getchar();

CID 1412794 points out that this assigns the result
of getchar() to a uint8_t, which drops the distinction
between EOF and a legitimate byte.
CID 1412795 is then kind of a run-on error from that,
complaining that the int result from getchar() is
truncated before returning it.

I'm not sure what we should do with EOF, but presumably
we should handle it in some way.

thanks
-- PMM