Re: [Qemu-devel] [PATCH V3] Add stdio char device on windows

[Blue Swirl]

On Wed, Sep 28, 2011 at 4:28 PM, Fabien Chouteau <address@hidden> wrote:
> Simple implementation of an stdio char device on Windows.
>
> Signed-off-by: Fabien Chouteau <address@hidden>
> ---
>  qemu-char.c |  216 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 214 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 09d2309..0ec449b 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -538,6 +538,9 @@ int send_all(int fd, const void *_buf, int len1)
>  }
>  #endif /* !_WIN32 */
>
> +#define STDIO_MAX_CLIENTS 1
> +static int stdio_nb_clients;
> +
>  #ifndef _WIN32
>
>  typedef struct {
> @@ -545,8 +548,6 @@ typedef struct {
>     int max_size;
>  } FDCharDriver;
>
> -#define STDIO_MAX_CLIENTS 1
> -static int stdio_nb_clients = 0;
>
>  static int fd_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
>  {
> @@ -1451,6 +1452,8 @@ static int qemu_chr_open_pp(QemuOpts *opts, 
> CharDriverState **_chr)
>
>  #else /* _WIN32 */
>
> +static CharDriverState *stdio_clients[STDIO_MAX_CLIENTS];
> +
>  typedef struct {
>     int max_size;
>     HANDLE hcom, hrecv, hsend;
> @@ -1809,6 +1812,214 @@ static int qemu_chr_open_win_file_out(QemuOpts *opts, 
> CharDriverState **_chr)
>
>     return qemu_chr_open_win_file(fd_out, _chr);
>  }
> +
> +static int win_stdio_write(CharDriverState *chr, const uint8_t *buf, int len)
> +{
> +    HANDLE *hStdOut = GetStdHandle(STD_OUTPUT_HANDLE);
> +    DWORD   dwSize;
> +    int     len1;
> +
> +    len1 = len;
> +
> +    while (len1 > 0) {
> +        if (!WriteFile(hStdOut, buf, len1, &dwSize, NULL)) {
> +            break;
> +        }
> +        buf  += dwSize;
> +        len1 -= dwSize;
> +    }
> +
> +    return len - len1;
> +}
> +
> +static HANDLE *hStdIn;

I think you could avoid these variables by introducing a structure
that is passed as opaque in place of CharDriverState below.

> +
> +static void win_stdio_wait_func(void *opaque)
> +{
> +    CharDriverState *chr = opaque;
> +    INPUT_RECORD     buf[4];
> +    int              ret;
> +    DWORD            dwSize;
> +    int              i;
> +
> +    ret = ReadConsoleInput(hStdIn, buf, sizeof(buf)/sizeof(*buf), &dwSize);

For readability, please add spaces around '/'.

> +
> +    if (!ret) {
> +        /* Avoid error storm */
> +        qemu_del_wait_object(hStdIn, NULL, NULL);
> +        return;
> +    }
> +
> +    for (i = 0; i < dwSize; i++) {
> +        KEY_EVENT_RECORD *kev = &buf[i].Event.KeyEvent;
> +
> +        if (buf[i].EventType == KEY_EVENT && kev->bKeyDown) {
> +            int j;
> +            if (kev->uChar.AsciiChar != 0) {
> +                for (j = 0; j < kev->wRepeatCount; j++)

Missing braces.

> +                    if (qemu_chr_be_can_write(chr)) {
> +                        uint8_t c = kev->uChar.AsciiChar;
> +                        qemu_chr_be_write(chr, &c, 1);
> +                    }
> +            }
> +        }
> +    }
> +}
> +
> +static HANDLE  hInputReadyEvent;
> +static HANDLE  hInputDoneEvent;
> +static HANDLE  hInputThread;
> +static uint8_t win_stdio_buf;
> +
> +static DWORD WINAPI win_stdio_thread(LPVOID param)
> +{
> +    int   ret;
> +    DWORD dwSize;
> +
> +    while (1) {
> +
> +        /* Wait for one byte */
> +        ret = ReadFile(hStdIn, &win_stdio_buf, 1, &dwSize, NULL);
> +
> +        /* Exit in case of error, continue if nothing read */
> +        if (!ret) {
> +            break;
> +        }
> +        if (!dwSize) {
> +            continue;
> +        }
> +
> +        /* Some terminal emulator returns \r\n for Enter, just pass \n */
> +        if (win_stdio_buf == '\r') {
> +            continue;
> +        }
> +
> +        /* Signal the main thread and wait until the byte was eaten */
> +        if (!SetEvent(hInputReadyEvent)) {
> +            break;
> +        }
> +        if (WaitForSingleObject(hInputDoneEvent, INFINITE) != WAIT_OBJECT_0) 
> {
> +            break;
> +        }
> +    }
> +
> +    qemu_del_wait_object(hInputReadyEvent, NULL, NULL);
> +    return 0;
> +}
> +
> +static void win_stdio_thread_wait_func(void *opaque)
> +{
> +    CharDriverState *chr = opaque;
> +
> +    if (qemu_chr_be_can_write(chr)) {
> +        qemu_chr_be_write(chr, &win_stdio_buf, 1);
> +    }
> +
> +    SetEvent(hInputDoneEvent);
> +}
> +
> +static void qemu_chr_set_echo_win_stdio(CharDriverState *chr, bool echo)
> +{
> +    DWORD dwMode = 0;
> +
> +    GetConsoleMode(hStdIn, &dwMode);
> +
> +    if (echo) {
> +        SetConsoleMode(hStdIn, dwMode | ENABLE_ECHO_INPUT);
> +    } else {
> +        SetConsoleMode(hStdIn, dwMode & ~ENABLE_ECHO_INPUT);
> +    }
> +}
> +
> +static void win_stdio_close(CharDriverState *chr)
> +{
> +    if (hInputReadyEvent != INVALID_HANDLE_VALUE) {
> +        CloseHandle(hInputReadyEvent);
> +    }
> +    if (hInputDoneEvent != INVALID_HANDLE_VALUE) {
> +        CloseHandle(hInputDoneEvent);
> +    }
> +    if (hInputThread != INVALID_HANDLE_VALUE) {
> +        TerminateThread(hInputThread, 0);
> +    }
> +
> +    g_free(chr);
> +    stdio_nb_clients--;
> +}
> +
> +static int qemu_chr_open_win_stdio(QemuOpts         *opts,

Extra spaces here.

> +                                                CharDriverState **_chr)
> +{
> +    CharDriverState *chr;
> +    DWORD            dwMode;
> +    int              is_console = 0;
> +
> +    hStdIn = GetStdHandle(STD_INPUT_HANDLE);
> +    if (hStdIn == INVALID_HANDLE_VALUE) {
> +        fprintf(stderr, "cannot open stdio: invalid handle\n");
> +        exit(1);
> +    }
> +
> +    is_console = GetConsoleMode(hStdIn, &dwMode) != 0;
> +
> +    if (stdio_nb_clients >= STDIO_MAX_CLIENTS
> +        || ((display_type != DT_NOGRAPHIC) && (stdio_nb_clients != 0))) {
> +        return -EIO;
> +    }
> +
> +    chr = g_malloc0(sizeof(CharDriverState));
> +    if (!chr) {


g_malloc0() will abort if allocation fails, so the check is useless.

> +        return -ENOMEM;
> +    }
> +
> +    chr->chr_write = win_stdio_write;
> +    chr->chr_close = win_stdio_close;
> +
> +    if (stdio_nb_clients == 0) {
> +        if (is_console) {
> +            if (qemu_add_wait_object(hStdIn,
> +                                     win_stdio_wait_func, chr)) {
> +                fprintf(stderr, "qemu_add_wait_object: failed\n");
> +            }
> +        } else {
> +            DWORD   dwId;
> +
> +            hInputReadyEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
> +            hInputDoneEvent  = CreateEvent(NULL, FALSE, FALSE, NULL);
> +            hInputThread     = CreateThread(NULL, 0, win_stdio_thread,
> +                                            chr, 0, &dwId);
> +
> +            if (   hInputThread     == INVALID_HANDLE_VALUE

Please use checkpatch.pl to avoid common issues:
ERROR: space prohibited after that open parenthesis '('
#286: FILE: qemu-char.c:1992:
+            if (   hInputThread     == INVALID_HANDLE_VALUE

total: 1 errors, 0 warnings, 246 lines checked


> +                || hInputReadyEvent == INVALID_HANDLE_VALUE
> +                || hInputDoneEvent  == INVALID_HANDLE_VALUE) {
> +                fprintf(stderr, "cannot create stdio thread or event\n");
> +                exit(1);
> +            }
> +            if (qemu_add_wait_object(hInputReadyEvent,
> +                                     win_stdio_thread_wait_func, chr)) {
> +                fprintf(stderr, "qemu_add_wait_object: failed\n");
> +            }
> +        }
> +    }
> +
> +    dwMode |= ENABLE_LINE_INPUT;
> +
> +    stdio_clients[stdio_nb_clients++] = chr;
> +    if (stdio_nb_clients == 1 && is_console) {
> +        /* set the terminal in raw mode */
> +        /* ENABLE_QUICK_EDIT_MODE | ENABLE_EXTENDED_FLAGS */
> +        dwMode |= ENABLE_PROCESSED_INPUT;
> +    }
> +
> +    SetConsoleMode(hStdIn, dwMode);
> +
> +    chr->chr_set_echo = qemu_chr_set_echo_win_stdio;
> +    qemu_chr_fe_set_echo(chr, false);
> +
> +    *_chr = chr;
> +
> +    return 0;
> +}
>  #endif /* !_WIN32 */
>
>  /***********************************************************/
> @@ -2519,6 +2730,7 @@ static const struct {
>     { .name = "pipe",      .open = qemu_chr_open_win_pipe },
>     { .name = "console",   .open = qemu_chr_open_win_con },
>     { .name = "serial",    .open = qemu_chr_open_win },
> +    { .name = "stdio",     .open = qemu_chr_open_win_stdio },
>  #else
>     { .name = "file",      .open = qemu_chr_open_file_out },
>     { .name = "pipe",      .open = qemu_chr_open_pipe },
> --
> 1.7.4.1
>
>
>