[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 5/6] char: change qemu_chr_fe_add_watch to retur
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 5/6] char: change qemu_chr_fe_add_watch to return unsigned |
Date: |
Wed, 22 Jun 2016 16:22:02 +0100 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
* Paolo Bonzini (address@hidden) wrote:
> g_source_attach can return any value between 1 and UINT_MAX if you let
> QEMU run long enough. However, qemu_chr_fe_add_watch can also return
> a negative errno value when the device is disconnected or does not
> support chr_add_watch. Change it to return zero to avoid overloading
> these values.
>
> Fix the cadence_uart which asserts in this case (easily obtained with
> "-serial pty").
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> hw/char/cadence_uart.c | 5 ++++-
> include/sysemu/char.h | 16 ++++++++++++++--
> qemu-char.c | 8 ++++----
> 3 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index c856fc3..488a570 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -294,7 +294,10 @@ static gboolean cadence_uart_xmit(GIOChannel *chan,
> GIOCondition cond,
> if (s->tx_count) {
> int r = qemu_chr_fe_add_watch(s->chr, G_IO_OUT|G_IO_HUP,
> cadence_uart_xmit, s);
> - assert(r);
> + if (!r) {
> + s->tx_count = 0;
> + return FALSE;
> + }
> }
>
> uart_update_status(s);
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index 1eb2d0f..07434a0 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -221,8 +221,20 @@ void qemu_chr_fe_event(CharDriverState *s, int event);
> void qemu_chr_fe_printf(CharDriverState *s, const char *fmt, ...)
> GCC_FMT_ATTR(2, 3);
>
> -int qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond,
> - GIOFunc func, void *user_data);
> +/**
> + * @qemu_chr_fe_add_watch:
> + *
> + * If the backend is connected, create and add a #GSource that fires
> + * when the given condition (typically G_IO_OUT|G_IO_HUP or G_IO_HUP)
> + * is active; return the #GSource's tag. If it is disconnected,
> + * return 0.
> + *
> + * @cond the condition to poll for
> + * @func the function to call when the condition happens
> + * @user_data the opaque pointer to pass to @func
> + */
> +unsigned qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond,
> + GIOFunc func, void *user_data);
can we make this a guint? hw/char/virtio-console.c and monitor.c already
use guint as the type for a watch and that matches the glib definition?
(Although not net/vhost-user.c which still has int, but that should probably
be fixed anyway).
But other than that;
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Dave
>
> /**
> * @qemu_chr_fe_write:
> diff --git a/qemu-char.c b/qemu-char.c
> index 84f49ac..39b2ccd 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3966,19 +3966,19 @@ void qemu_chr_fe_event(struct CharDriverState *chr,
> int event)
> }
> }
>
> -int qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond,
> - GIOFunc func, void *user_data)
> +unsigned qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond,
> + GIOFunc func, void *user_data)
> {
> GSource *src;
> guint tag;
>
> if (s->chr_add_watch == NULL) {
> - return -ENOSYS;
> + return 0;
> }
>
> src = s->chr_add_watch(s, cond);
> if (!src) {
> - return -EINVAL;
> + return 0;
> }
>
> g_source_set_callback(src, (GSourceFunc)func, user_data, NULL);
> --
> 2.5.5
>
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK