From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:39934) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rumfa-0002Jh-PC for qemu-devel@nongnu.org; Tue, 07 Feb 2012 10:09:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RumfW-00040R-7X for qemu-devel@nongnu.org; Tue, 07 Feb 2012 10:09:22 -0500 Received: from e31.co.us.ibm.com ([32.97.110.149]:36372) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RumfV-0003yu-Nu for qemu-devel@nongnu.org; Tue, 07 Feb 2012 10:09:18 -0500 Received: from /spool/local by e31.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 7 Feb 2012 08:09:08 -0700 Received: from d03relay03.boulder.ibm.com (d03relay03.boulder.ibm.com [9.17.195.228]) by d03dlp03.boulder.ibm.com (Postfix) with ESMTP id 0DFBD19D8048 for ; Tue, 7 Feb 2012 08:07:29 -0700 (MST) Received: from d03av05.boulder.ibm.com (d03av05.boulder.ibm.com [9.17.195.85]) by d03relay03.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q17F7Gs6067870 for ; Tue, 7 Feb 2012 08:07:32 -0700 Received: from d03av05.boulder.ibm.com (loopback [127.0.0.1]) by d03av05.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q17F6pUc016745 for ; Tue, 7 Feb 2012 08:06:51 -0700 Message-ID: <4F313E02.1060701@us.ibm.com> Date: Tue, 07 Feb 2012 09:06:42 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1328623766-12287-1-git-send-email-armbru@redhat.com> <1328623766-12287-2-git-send-email-armbru@redhat.com> In-Reply-To: <1328623766-12287-2-git-send-email-armbru@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 01/19] Revert "qemu-char: Print strerror message on failure" and deps List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, qemu-devel@nongnu.org On 02/07/2012 08:09 AM, Markus Armbruster wrote: > The commit's purpose is laudable: > > The only way for chardev drivers to communicate an error was to > return a NULL pointer, which resulted in an error message that > said _that_ something went wrong, but not _why_. > > It attempts to achieve it by changing the interface to return 0/-errno > and update qemu_chr_open_opts() to use strerror() to display a more > helpful error message. Unfortunately, it has serious flaws: > > 1. Backends "socket" and "udp" return bogus error codes, because > qemu_chr_open_socket() and qemu_chr_open_udp() assume that > unix_listen_opts(), unix_connect_opts(), inet_listen_opts(), > inet_connect_opts() and inet_dgram_opts() fail with errno set > appropriately. That assumption is wrong, and the commit turns > unspecific error messages into misleading error messages. For > instance: > > $ qemu-system-x86_64 -nodefaults -vnc :0 -chardev socket,id=bar,host=xxx > inet_connect: host and/or port not specified > chardev: opening backend "socket" failed: No such file or directory > > ENOENT is what happens to be in my errno when the backend returns > -errno. Let's put ERANGE there just for giggles: > > $ qemu-system-x86_64 -nodefaults -vnc :0 -chardev socket,id=bar,host=xxx -drive if=none,iops=99999999999999999999 > inet_connect: host and/or port not specified > chardev: opening backend "socket" failed: Numerical result out of range > > Worse: when errno happens to be zero, return -errno erroneously > signals success, and qemu_chr_new_from_opts() dies dereferencing > uninitialized chr. I observe this with "-serial unix:". > > 2. All qemu_chr_open_opts() knows about the error is an errno error > code. That's simply not enough for a decent message. For instance, > when inet_dgram() can't resolve the parameter host, which errno code > should it use? What if it can't resolve parameter localaddr? > > Clue: many backends already report errors in their open methods. > Let's revert the flawed commit along with its dependencies, and fix up > the silent error paths instead. > > This reverts commit 6e1db57b2ac9025c2443c665a0d9e78748637b26. Ack on this, but see comments in other patches for what I think we should do instead. Regards, Anthony Liguori > > Conflicts: > > console.c > hw/baum.c > qemu-char.c > > This reverts commit aad04cd024f0c59f0b96f032cde2e24eb3abba6d. > > The parts of commit db418a0a "Add stdio char device on windows" that > depend on the reverted change fixed up. > > Signed-off-by: Markus Armbruster > --- > console.c | 8 +-- > console.h | 2 +- > hw/baum.c | 7 +- > hw/baum.h | 2 +- > hw/msmouse.c | 5 +- > hw/msmouse.h | 2 +- > qemu-char.c | 169 +++++++++++++++++++++------------------------------- > spice-qemu-char.c | 9 +-- > ui/qemu-spice.h | 2 +- > 9 files changed, 84 insertions(+), 122 deletions(-) > > diff --git a/console.c b/console.c > index 135394f..2c432e3 100644 > --- a/console.c > +++ b/console.c > @@ -1510,7 +1510,7 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds) > chr->init(chr); > } > > -int text_console_init(QemuOpts *opts, CharDriverState **_chr) > +CharDriverState *text_console_init(QemuOpts *opts) > { > CharDriverState *chr; > TextConsole *s; > @@ -1542,7 +1542,7 @@ int text_console_init(QemuOpts *opts, CharDriverState **_chr) > > if (!s) { > g_free(chr); > - return -EBUSY; > + return NULL; > } > > s->chr = chr; > @@ -1550,9 +1550,7 @@ int text_console_init(QemuOpts *opts, CharDriverState **_chr) > s->g_height = height; > chr->opaque = s; > chr->chr_set_echo = text_console_set_echo; > - > - *_chr = chr; > - return 0; > + return chr; > } > > void text_consoles_set_display(DisplayState *ds) > diff --git a/console.h b/console.h > index 6ba0d5d..c33ffe0 100644 > --- a/console.h > +++ b/console.h > @@ -356,7 +356,7 @@ void vga_hw_text_update(console_ch_t *chardata); > > int is_graphic_console(void); > int is_fixedsize_console(void); > -int text_console_init(QemuOpts *opts, CharDriverState **_chr); > +CharDriverState *text_console_init(QemuOpts *opts); > void text_consoles_set_display(DisplayState *ds); > void console_select(unsigned int index); > void console_color_init(DisplayState *ds); > diff --git a/hw/baum.c b/hw/baum.c > index 86d780a..3e94f84 100644 > --- a/hw/baum.c > +++ b/hw/baum.c > @@ -562,7 +562,7 @@ static void baum_close(struct CharDriverState *chr) > g_free(baum); > } > > -int chr_baum_init(QemuOpts *opts, CharDriverState **_chr) > +CharDriverState *chr_baum_init(QemuOpts *opts) > { > BaumDriverState *baum; > CharDriverState *chr; > @@ -614,8 +614,7 @@ int chr_baum_init(QemuOpts *opts, CharDriverState **_chr) > > qemu_chr_generic_open(chr); > > - *_chr = chr; > - return 0; > + return chr; > > fail: > qemu_free_timer(baum->cellCount_timer); > @@ -624,5 +623,5 @@ fail_handle: > g_free(handle); > g_free(chr); > g_free(baum); > - return -EIO; > + return NULL; > } > diff --git a/hw/baum.h b/hw/baum.h > index 3f28cc3..8af710f 100644 > --- a/hw/baum.h > +++ b/hw/baum.h > @@ -23,4 +23,4 @@ > */ > > /* char device */ > -int chr_baum_init(QemuOpts *opts, CharDriverState **_chr); > +CharDriverState *chr_baum_init(QemuOpts *opts); > diff --git a/hw/msmouse.c b/hw/msmouse.c > index c3b57ea..9c492a4 100644 > --- a/hw/msmouse.c > +++ b/hw/msmouse.c > @@ -64,7 +64,7 @@ static void msmouse_chr_close (struct CharDriverState *chr) > g_free (chr); > } > > -int qemu_chr_open_msmouse(QemuOpts *opts, CharDriverState **_chr) > +CharDriverState *qemu_chr_open_msmouse(QemuOpts *opts) > { > CharDriverState *chr; > > @@ -74,6 +74,5 @@ int qemu_chr_open_msmouse(QemuOpts *opts, CharDriverState **_chr) > > qemu_add_mouse_event_handler(msmouse_event, chr, 0, "QEMU Microsoft Mouse"); > > - *_chr = chr; > - return 0; > + return chr; > } > diff --git a/hw/msmouse.h b/hw/msmouse.h > index 8b853b3..456cb21 100644 > --- a/hw/msmouse.h > +++ b/hw/msmouse.h > @@ -1,2 +1,2 @@ > /* msmouse.c */ > -int qemu_chr_open_msmouse(QemuOpts *opts, CharDriverState **_chr); > +CharDriverState *qemu_chr_open_msmouse(QemuOpts *opts); > diff --git a/qemu-char.c b/qemu-char.c > index b1d80dd..1e882cf 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -218,15 +218,13 @@ static int null_chr_write(CharDriverState *chr, const uint8_t *buf, int len) > return len; > } > > -static int qemu_chr_open_null(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_null(QemuOpts *opts) > { > CharDriverState *chr; > > chr = g_malloc0(sizeof(CharDriverState)); > chr->chr_write = null_chr_write; > - > - *_chr= chr; > - return 0; > + return chr; > } > > /* MUX driver for serial I/O splitting */ > @@ -636,21 +634,18 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int fd_out) > return chr; > } > > -static int qemu_chr_open_file_out(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_file_out(QemuOpts *opts) > { > int fd_out; > > TFR(fd_out = qemu_open(qemu_opt_get(opts, "path"), > O_WRONLY | O_TRUNC | O_CREAT | O_BINARY, 0666)); > - if (fd_out< 0) { > - return -errno; > - } > - > - *_chr = qemu_chr_open_fd(-1, fd_out); > - return 0; > + if (fd_out< 0) > + return NULL; > + return qemu_chr_open_fd(-1, fd_out); > } > > -static int qemu_chr_open_pipe(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_pipe(QemuOpts *opts) > { > int fd_in, fd_out; > char filename_in[256], filename_out[256]; > @@ -658,7 +653,7 @@ static int qemu_chr_open_pipe(QemuOpts *opts, CharDriverState **_chr) > > if (filename == NULL) { > fprintf(stderr, "chardev: pipe: no filename given\n"); > - return -EINVAL; > + return NULL; > } > > snprintf(filename_in, 256, "%s.in", filename); > @@ -670,14 +665,11 @@ static int qemu_chr_open_pipe(QemuOpts *opts, CharDriverState **_chr) > close(fd_in); > if (fd_out>= 0) > close(fd_out); > - TFR(fd_in = fd_out = qemu_open(filename, O_RDWR | O_BINARY)); > - if (fd_in< 0) { > - return -errno; > - } > + TFR(fd_in = fd_out = open(filename, O_RDWR | O_BINARY)); > + if (fd_in< 0) > + return NULL; > } > - > - *_chr = qemu_chr_open_fd(fd_in, fd_out); > - return 0; > + return qemu_chr_open_fd(fd_in, fd_out); > } > > > @@ -768,14 +760,12 @@ static void qemu_chr_close_stdio(struct CharDriverState *chr) > fd_chr_close(chr); > } > > -static int qemu_chr_open_stdio(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_stdio(QemuOpts *opts) > { > CharDriverState *chr; > > - if (stdio_nb_clients>= STDIO_MAX_CLIENTS) { > - return -EBUSY; > - } > - > + if (stdio_nb_clients>= STDIO_MAX_CLIENTS) > + return NULL; > if (stdio_nb_clients == 0) { > old_fd0_flags = fcntl(0, F_GETFL); > tcgetattr (0,&oldtty); > @@ -792,8 +782,7 @@ static int qemu_chr_open_stdio(QemuOpts *opts, CharDriverState **_chr) > display_type != DT_NOGRAPHIC); > qemu_chr_fe_set_echo(chr, false); > > - *_chr = chr; > - return 0; > + return chr; > } > > #ifdef __sun__ > @@ -980,7 +969,7 @@ static void pty_chr_close(struct CharDriverState *chr) > qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > } > > -static int qemu_chr_open_pty(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_pty(QemuOpts *opts) > { > CharDriverState *chr; > PtyCharDriver *s; > @@ -995,7 +984,7 @@ static int qemu_chr_open_pty(QemuOpts *opts, CharDriverState **_chr) > #endif > > if (openpty(&master_fd,&slave_fd, pty_name, NULL, NULL)< 0) { > - return -errno; > + return NULL; > } > > /* Set raw attributes on the pty. */ > @@ -1021,8 +1010,7 @@ static int qemu_chr_open_pty(QemuOpts *opts, CharDriverState **_chr) > s->fd = master_fd; > s->timer = qemu_new_timer_ms(rt_clock, pty_chr_timer, chr); > > - *_chr = chr; > - return 0; > + return chr; > } > > static void tty_serial_init(int fd, int speed, > @@ -1223,28 +1211,30 @@ static void qemu_chr_close_tty(CharDriverState *chr) > } > } > > -static int qemu_chr_open_tty(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_tty(QemuOpts *opts) > { > const char *filename = qemu_opt_get(opts, "path"); > CharDriverState *chr; > int fd; > > - TFR(fd = qemu_open(filename, O_RDWR | O_NONBLOCK)); > + TFR(fd = open(filename, O_RDWR | O_NONBLOCK)); > if (fd< 0) { > - return -errno; > + return NULL; > } > tty_serial_init(fd, 115200, 'N', 8, 1); > chr = qemu_chr_open_fd(fd, fd); > + if (!chr) { > + close(fd); > + return NULL; > + } > chr->chr_ioctl = tty_serial_ioctl; > chr->chr_close = qemu_chr_close_tty; > - > - *_chr = chr; > - return 0; > + return chr; > } > #else /* ! __linux__&& ! __sun__ */ > -static int qemu_chr_open_pty(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_pty(QemuOpts *opts) > { > - return -ENOTSUP; > + return NULL; > } > #endif /* __linux__ || __sun__ */ > > @@ -1358,7 +1348,7 @@ static void pp_close(CharDriverState *chr) > qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > } > > -static int qemu_chr_open_pp(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_pp(QemuOpts *opts) > { > const char *filename = qemu_opt_get(opts, "path"); > CharDriverState *chr; > @@ -1366,13 +1356,12 @@ static int qemu_chr_open_pp(QemuOpts *opts, CharDriverState **_chr) > int fd; > > TFR(fd = open(filename, O_RDWR)); > - if (fd< 0) { > - return -errno; > - } > + if (fd< 0) > + return NULL; > > if (ioctl(fd, PPCLAIM)< 0) { > close(fd); > - return -errno; > + return NULL; > } > > drv = g_malloc0(sizeof(ParallelCharDriver)); > @@ -1387,8 +1376,7 @@ static int qemu_chr_open_pp(QemuOpts *opts, CharDriverState **_chr) > > qemu_chr_generic_open(chr); > > - *_chr = chr; > - return 0; > + return chr; > } > #endif /* __linux__ */ > > @@ -1430,24 +1418,21 @@ static int pp_ioctl(CharDriverState *chr, int cmd, void *arg) > return 0; > } > > -static int qemu_chr_open_pp(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_pp(QemuOpts *opts) > { > const char *filename = qemu_opt_get(opts, "path"); > CharDriverState *chr; > int fd; > > - fd = qemu_open(filename, O_RDWR); > - if (fd< 0) { > - return -errno; > - } > + fd = open(filename, O_RDWR); > + if (fd< 0) > + return NULL; > > chr = g_malloc0(sizeof(CharDriverState)); > chr->opaque = (void *)(intptr_t)fd; > chr->chr_write = null_chr_write; > chr->chr_ioctl = pp_ioctl; > - > - *_chr = chr; > - return 0; > + return chr; > } > #endif > > @@ -1663,7 +1648,7 @@ static int win_chr_poll(void *opaque) > return 0; > } > > -static int qemu_chr_open_win(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_win(QemuOpts *opts) > { > const char *filename = qemu_opt_get(opts, "path"); > CharDriverState *chr; > @@ -1678,12 +1663,10 @@ static int qemu_chr_open_win(QemuOpts *opts, CharDriverState **_chr) > if (win_chr_init(chr, filename)< 0) { > g_free(s); > g_free(chr); > - return -EIO; > + return NULL; > } > qemu_chr_generic_open(chr); > - > - *_chr = chr; > - return 0; > + return chr; > } > > static int win_chr_pipe_poll(void *opaque) > @@ -1765,7 +1748,7 @@ static int win_chr_pipe_init(CharDriverState *chr, const char *filename) > } > > > -static int qemu_chr_open_win_pipe(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_win_pipe(QemuOpts *opts) > { > const char *filename = qemu_opt_get(opts, "path"); > CharDriverState *chr; > @@ -1780,15 +1763,13 @@ static int qemu_chr_open_win_pipe(QemuOpts *opts, CharDriverState **_chr) > if (win_chr_pipe_init(chr, filename)< 0) { > g_free(s); > g_free(chr); > - return -EIO; > + return NULL; > } > qemu_chr_generic_open(chr); > - > - *_chr = chr; > - return 0; > + return chr; > } > > -static int qemu_chr_open_win_file(HANDLE fd_out, CharDriverState **pchr) > +static CharDriverState *qemu_chr_open_win_file(HANDLE fd_out) > { > CharDriverState *chr; > WinCharState *s; > @@ -1799,27 +1780,25 @@ static int qemu_chr_open_win_file(HANDLE fd_out, CharDriverState **pchr) > chr->opaque = s; > chr->chr_write = win_chr_write; > qemu_chr_generic_open(chr); > - *pchr = chr; > - return 0; > + return chr; > } > > -static int qemu_chr_open_win_con(QemuOpts *opts, CharDriverState **chr) > +static CharDriverState *qemu_chr_open_win_con(QemuOpts *opts) > { > - return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE), chr); > + return qemu_chr_open_win_file(GetStdHandle(STD_OUTPUT_HANDLE)); > } > > -static int qemu_chr_open_win_file_out(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_win_file_out(QemuOpts *opts) > { > const char *file_out = qemu_opt_get(opts, "path"); > HANDLE fd_out; > > fd_out = CreateFile(file_out, GENERIC_WRITE, FILE_SHARE_READ, NULL, > OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); > - if (fd_out == INVALID_HANDLE_VALUE) { > - return -EIO; > - } > + if (fd_out == INVALID_HANDLE_VALUE) > + return NULL; > > - return qemu_chr_open_win_file(fd_out, _chr); > + return qemu_chr_open_win_file(fd_out); > } > > static int win_stdio_write(CharDriverState *chr, const uint8_t *buf, int len) > @@ -1960,7 +1939,7 @@ static void win_stdio_close(CharDriverState *chr) > stdio_nb_clients--; > } > > -static int qemu_chr_open_win_stdio(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_win_stdio(QemuOpts *opts) > { > CharDriverState *chr; > WinStdioCharState *stdio; > @@ -1969,7 +1948,7 @@ static int qemu_chr_open_win_stdio(QemuOpts *opts, CharDriverState **_chr) > > if (stdio_nb_clients>= STDIO_MAX_CLIENTS > || ((display_type != DT_NOGRAPHIC)&& (stdio_nb_clients != 0))) { > - return -EIO; > + return NULL; > } > > chr = g_malloc0(sizeof(CharDriverState)); > @@ -2028,9 +2007,7 @@ static int qemu_chr_open_win_stdio(QemuOpts *opts, CharDriverState **_chr) > chr->chr_set_echo = qemu_chr_set_echo_win_stdio; > qemu_chr_fe_set_echo(chr, false); > > - *_chr = chr; > - > - return 0; > + return chr; > } > #endif /* !_WIN32 */ > > @@ -2111,12 +2088,11 @@ static void udp_chr_close(CharDriverState *chr) > qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > } > > -static int qemu_chr_open_udp(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_udp(QemuOpts *opts) > { > CharDriverState *chr = NULL; > NetCharDriver *s = NULL; > int fd = -1; > - int ret; > > chr = g_malloc0(sizeof(CharDriverState)); > s = g_malloc0(sizeof(NetCharDriver)); > @@ -2124,7 +2100,6 @@ static int qemu_chr_open_udp(QemuOpts *opts, CharDriverState **_chr) > fd = inet_dgram_opts(opts); > if (fd< 0) { > fprintf(stderr, "inet_dgram_opts failed\n"); > - ret = -errno; > goto return_err; > } > > @@ -2135,9 +2110,7 @@ static int qemu_chr_open_udp(QemuOpts *opts, CharDriverState **_chr) > chr->chr_write = udp_chr_write; > chr->chr_update_read_handler = udp_chr_update_read_handler; > chr->chr_close = udp_chr_close; > - > - *_chr = chr; > - return 0; > + return chr; > > return_err: > g_free(chr); > @@ -2145,7 +2118,7 @@ return_err: > if (fd>= 0) { > closesocket(fd); > } > - return ret; > + return NULL; > } > > /***********************************************************/ > @@ -2436,7 +2409,7 @@ static void tcp_chr_close(CharDriverState *chr) > qemu_chr_be_event(chr, CHR_EVENT_CLOSED); > } > > -static int qemu_chr_open_socket(QemuOpts *opts, CharDriverState **_chr) > +static CharDriverState *qemu_chr_open_socket(QemuOpts *opts) > { > CharDriverState *chr = NULL; > TCPCharDriver *s = NULL; > @@ -2446,7 +2419,6 @@ static int qemu_chr_open_socket(QemuOpts *opts, CharDriverState **_chr) > int do_nodelay; > int is_unix; > int is_telnet; > - int ret; > > is_listen = qemu_opt_get_bool(opts, "server", 0); > is_waitconnect = qemu_opt_get_bool(opts, "wait", 1); > @@ -2472,10 +2444,8 @@ static int qemu_chr_open_socket(QemuOpts *opts, CharDriverState **_chr) > fd = inet_connect_opts(opts); > } > } > - if (fd< 0) { > - ret = -errno; > + if (fd< 0) > goto fail; > - } > > if (!is_waitconnect) > socket_set_nonblock(fd); > @@ -2528,16 +2498,14 @@ static int qemu_chr_open_socket(QemuOpts *opts, CharDriverState **_chr) > tcp_chr_accept(chr); > socket_set_nonblock(s->listen_fd); > } > - > - *_chr = chr; > - return 0; > + return chr; > > fail: > if (fd>= 0) > closesocket(fd); > g_free(s); > g_free(chr); > - return ret; > + return NULL; > } > > /***********************************************************/ > @@ -2730,7 +2698,7 @@ fail: > > static const struct { > const char *name; > - int (*open)(QemuOpts *opts, CharDriverState **chr); > + CharDriverState *(*open)(QemuOpts *opts); > } backend_table[] = { > { .name = "null", .open = qemu_chr_open_null }, > { .name = "socket", .open = qemu_chr_open_socket }, > @@ -2771,7 +2739,6 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, > { > CharDriverState *chr; > int i; > - int ret; > > if (qemu_opts_id(opts) == NULL) { > fprintf(stderr, "chardev: no id specified\n"); > @@ -2793,10 +2760,10 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, > return NULL; > } > > - ret = backend_table[i].open(opts,&chr); > - if (ret< 0) { > - fprintf(stderr, "chardev: opening backend \"%s\" failed: %s\n", > - qemu_opt_get(opts, "backend"), strerror(-ret)); > + chr = backend_table[i].open(opts); > + if (!chr) { > + fprintf(stderr, "chardev: opening backend \"%s\" failed\n", > + qemu_opt_get(opts, "backend")); > return NULL; > } > > diff --git a/spice-qemu-char.c b/spice-qemu-char.c > index 7e8eaa9..1e735ff 100644 > --- a/spice-qemu-char.c > +++ b/spice-qemu-char.c > @@ -188,7 +188,7 @@ static void print_allowed_subtypes(void) > fprintf(stderr, "\n"); > } > > -int qemu_chr_open_spice(QemuOpts *opts, CharDriverState **_chr) > +CharDriverState *qemu_chr_open_spice(QemuOpts *opts) > { > CharDriverState *chr; > SpiceCharDriver *s; > @@ -200,7 +200,7 @@ int qemu_chr_open_spice(QemuOpts *opts, CharDriverState **_chr) > if (name == NULL) { > fprintf(stderr, "spice-qemu-char: missing name parameter\n"); > print_allowed_subtypes(); > - return -EINVAL; > + return NULL; > } > for(;*psubtype != NULL; ++psubtype) { > if (strcmp(name, *psubtype) == 0) { > @@ -211,7 +211,7 @@ int qemu_chr_open_spice(QemuOpts *opts, CharDriverState **_chr) > if (subtype == NULL) { > fprintf(stderr, "spice-qemu-char: unsupported name\n"); > print_allowed_subtypes(); > - return -EINVAL; > + return NULL; > } > > chr = g_malloc0(sizeof(CharDriverState)); > @@ -233,6 +233,5 @@ int qemu_chr_open_spice(QemuOpts *opts, CharDriverState **_chr) > } > #endif > > - *_chr = chr; > - return 0; > + return chr; > } > diff --git a/ui/qemu-spice.h b/ui/qemu-spice.h > index c35b29c..a6c4a2c 100644 > --- a/ui/qemu-spice.h > +++ b/ui/qemu-spice.h > @@ -44,7 +44,7 @@ int qemu_spice_migrate_info(const char *hostname, int port, int tls_port, > void do_info_spice_print(Monitor *mon, const QObject *data); > void do_info_spice(Monitor *mon, QObject **ret_data); > > -int qemu_chr_open_spice(QemuOpts *opts, CharDriverState **_chr); > +CharDriverState *qemu_chr_open_spice(QemuOpts *opts); > > #else /* CONFIG_SPICE */ > #include "monitor.h"