| Message ID | 1466432945-28682-6-git-send-email-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Paolo Bonzini (pbonzini@redhat.com) 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 <pbonzini@redhat.com> > --- > 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 <dgilbert@redhat.com> 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 / dgilbert@redhat.com / Manchester, UK
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); /** * @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);
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 <pbonzini@redhat.com> --- hw/char/cadence_uart.c | 5 ++++- include/sysemu/char.h | 16 ++++++++++++++-- qemu-char.c | 8 ++++---- 3 files changed, 22 insertions(+), 7 deletions(-)