[Qemu-devel] [PATCH 11/19] qemu-char: Chardev open error reporting, _WIN32 part

[Markus Armbruster]

Convert backends "pipe", "console" and "serial" to error_report().
While there, improve the atrocious error messages some.

Unlike many other backends, "file" and "stdio" leave open error
reporting to their caller.  Because the caller doesn't know what went
wrong, this results in a pretty useless error message.  Change them to
report their errors.

Many error paths smell leaky, but I'm limiting myself strictly to
error reporting here.

Compile-tested only.

Signed-off-by: Markus Armbruster <address@hidden>
---
 qemu-char.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 51 insertions(+), 14 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 47bab4f..130ed8b 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1476,6 +1476,15 @@ typedef struct {
 static int win_chr_poll(void *opaque);
 static int win_chr_pipe_poll(void *opaque);
 
+static char *strerr_win(DWORD err)
+{
+    char **p;
+
+    FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_ALLOCATE_BUFFER,
+                  NULL, err, 0, (LPTSTR)&p, 0, NULL);
+    return *p;
+}
+
 static void win_chr_close(CharDriverState *chr)
 {
     WinCharState *s = chr->opaque;
@@ -1508,28 +1517,29 @@ static int win_chr_init(CharDriverState *chr, const 
char *filename)
     COMSTAT comstat;
     DWORD size;
     DWORD err;
+    char *errstr;
 
     s->hsend = CreateEvent(NULL, TRUE, FALSE, NULL);
     if (!s->hsend) {
-        fprintf(stderr, "Failed CreateEvent\n");
+        err = GetLastError();
         goto fail;
     }
     s->hrecv = CreateEvent(NULL, TRUE, FALSE, NULL);
     if (!s->hrecv) {
-        fprintf(stderr, "Failed CreateEvent\n");
+        err = GetLastError();
         goto fail;
     }
 
     s->hcom = CreateFile(filename, GENERIC_READ|GENERIC_WRITE, 0, NULL,
                       OPEN_EXISTING, FILE_FLAG_OVERLAPPED, 0);
     if (s->hcom == INVALID_HANDLE_VALUE) {
-        fprintf(stderr, "Failed CreateFile (%lu)\n", GetLastError());
+        err = GetLastError();
         s->hcom = NULL;
         goto fail;
     }
 
     if (!SetupComm(s->hcom, NRECVBUF, NSENDBUF)) {
-        fprintf(stderr, "Failed SetupComm\n");
+        err = GetLastError();
         goto fail;
     }
 
@@ -1540,29 +1550,32 @@ static int win_chr_init(CharDriverState *chr, const 
char *filename)
     CommConfigDialog(filename, NULL, &comcfg);
 
     if (!SetCommState(s->hcom, &comcfg.dcb)) {
-        fprintf(stderr, "Failed SetCommState\n");
+        err = GetLastError();
         goto fail;
     }
 
     if (!SetCommMask(s->hcom, EV_ERR)) {
-        fprintf(stderr, "Failed SetCommMask\n");
+        err = GetLastError();
         goto fail;
     }
 
     cto.ReadIntervalTimeout = MAXDWORD;
     if (!SetCommTimeouts(s->hcom, &cto)) {
-        fprintf(stderr, "Failed SetCommTimeouts\n");
+        err = GetLastError();
         goto fail;
     }
 
     if (!ClearCommError(s->hcom, &err, &comstat)) {
-        fprintf(stderr, "Failed ClearCommError\n");
+        err = GetLastError();
         goto fail;
     }
     qemu_add_polling_cb(win_chr_poll, chr);
     return 0;
 
  fail:
+    errstr = strerr_win(err);
+    error_report("Can't open serial device '%s': %s", filename, errstr);
+    LocalFree(errstr);
     win_chr_close(chr);
     return -1;
 }
@@ -1666,6 +1679,11 @@ static CharDriverState *qemu_chr_open_win(QemuOpts *opts)
     CharDriverState *chr;
     WinCharState *s;
 
+    if (!filename) {
+        error_report("serial character device requires parameter path");
+        return NULL;
+    }
+
     chr = g_malloc0(sizeof(CharDriverState));
     s = g_malloc0(sizeof(WinCharState));
     chr->opaque = s;
@@ -1702,19 +1720,20 @@ static int win_chr_pipe_init(CharDriverState *chr, 
const char *filename)
     WinCharState *s = chr->opaque;
     OVERLAPPED ov;
     int ret;
-    DWORD size;
+    DWORD size, err;
+    char *errstr;
     char openname[256];
 
     s->fpipe = TRUE;
 
     s->hsend = CreateEvent(NULL, TRUE, FALSE, NULL);
     if (!s->hsend) {
-        fprintf(stderr, "Failed CreateEvent\n");
+        err = GetLastError();
         goto fail;
     }
     s->hrecv = CreateEvent(NULL, TRUE, FALSE, NULL);
     if (!s->hrecv) {
-        fprintf(stderr, "Failed CreateEvent\n");
+        err = GetLastError();
         goto fail;
     }
 
@@ -1724,7 +1743,7 @@ static int win_chr_pipe_init(CharDriverState *chr, const 
char *filename)
                               PIPE_WAIT,
                               MAXCONNECT, NSENDBUF, NRECVBUF, NTIMEOUT, NULL);
     if (s->hcom == INVALID_HANDLE_VALUE) {
-        fprintf(stderr, "Failed CreateNamedPipe (%lu)\n", GetLastError());
+        err = GetLastError();
         s->hcom = NULL;
         goto fail;
     }
@@ -1733,13 +1752,13 @@ static int win_chr_pipe_init(CharDriverState *chr, 
const char *filename)
     ov.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
     ret = ConnectNamedPipe(s->hcom, &ov);
     if (ret) {
-        fprintf(stderr, "Failed ConnectNamedPipe\n");
+        err = GetLastError();
         goto fail;
     }
 
     ret = GetOverlappedResult(s->hcom, &ov, &size, TRUE);
     if (!ret) {
-        fprintf(stderr, "Failed GetOverlappedResult\n");
+        err = GetLastError();
         if (ov.hEvent) {
             CloseHandle(ov.hEvent);
             ov.hEvent = NULL;
@@ -1755,6 +1774,9 @@ static int win_chr_pipe_init(CharDriverState *chr, const 
char *filename)
     return 0;
 
  fail:
+    errstr = strerr_win(err);
+    error_report("Can't create pipe '%s': %s", filename, errstr);
+    LocalFree(errstr);
     win_chr_close(chr);
     return -1;
 }
@@ -1766,6 +1788,11 @@ static CharDriverState *qemu_chr_open_win_pipe(QemuOpts 
*opts)
     CharDriverState *chr;
     WinCharState *s;
 
+    if (!filename) {
+        error_report("pipe character device requires parameter path");
+        return NULL;
+    }
+
     chr = g_malloc0(sizeof(CharDriverState));
     s = g_malloc0(sizeof(WinCharState));
     chr->opaque = s;
@@ -1804,10 +1831,19 @@ static CharDriverState 
*qemu_chr_open_win_file_out(QemuOpts *opts)
 {
     const char *file_out = qemu_opt_get(opts, "path");
     HANDLE fd_out;
+    char *errstr;
+
+    if (!file_out) {
+        error_report("file character device requires parameter path");
+        return NULL;
+    }
 
     fd_out = CreateFile(file_out, GENERIC_WRITE, FILE_SHARE_READ, NULL,
                         OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL);
     if (fd_out == INVALID_HANDLE_VALUE) {
+        errstr = strerr_win(GetLastError());
+        error_report("Can't create file '%s': %s", file_out, errstr);
+        LocalFree(errstr);
         return NULL;
     }
 
@@ -1961,6 +1997,7 @@ static CharDriverState *qemu_chr_open_win_stdio(QemuOpts 
*opts)
 
     if (stdio_nb_clients >= STDIO_MAX_CLIENTS
         || ((display_type != DT_NOGRAPHIC) && (stdio_nb_clients != 0))) {
+        error_report("Too many stdio devices");
         return NULL;
     }
 
-- 
1.7.6.5