diff mbox series

[v2,05/11] chardev: mark explicitly first argument as poisoned

Message ID 20210804154848.557328-6-marcandre.lureau@redhat.com
State New
Headers show
Series chardev related fixes | expand

Commit Message

Marc-André Lureau Aug. 4, 2021, 3:48 p.m. UTC
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Since commit 9894dc0cdcc397ee5b26370bc53da6d360a363c2 "char: convert
from GIOChannel to QIOChannel", the first argument to the watch callback
can actually be a QIOChannel, which is not a GIOChannel (but a QEMU
Object).

Even though we never used that pointer, change the callback type to warn
the users. Possibly a better fix later, we may want to store the
callback and call it from intermediary functions.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/chardev/char-fe.h | 8 +++++++-
 chardev/char-fe.c         | 2 +-
 hw/char/cadence_uart.c    | 2 +-
 hw/char/cmsdk-apb-uart.c  | 2 +-
 hw/char/ibex_uart.c       | 2 +-
 hw/char/nrf51_uart.c      | 2 +-
 hw/char/serial.c          | 2 +-
 hw/char/virtio-console.c  | 2 +-
 hw/usb/redirect.c         | 2 +-
 hw/virtio/vhost-user.c    | 2 +-
 monitor/monitor.c         | 2 +-
 net/vhost-user.c          | 4 ++--
 12 files changed, 19 insertions(+), 13 deletions(-)

Comments

Daniel P. Berrangé Aug. 5, 2021, 10:36 a.m. UTC | #1
On Wed, Aug 04, 2021 at 07:48:42PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Since commit 9894dc0cdcc397ee5b26370bc53da6d360a363c2 "char: convert
> from GIOChannel to QIOChannel", the first argument to the watch callback
> can actually be a QIOChannel, which is not a GIOChannel (but a QEMU
> Object).
> 
> Even though we never used that pointer, change the callback type to warn
> the users. Possibly a better fix later, we may want to store the
> callback and call it from intermediary functions.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/chardev/char-fe.h | 8 +++++++-
>  chardev/char-fe.c         | 2 +-
>  hw/char/cadence_uart.c    | 2 +-
>  hw/char/cmsdk-apb-uart.c  | 2 +-
>  hw/char/ibex_uart.c       | 2 +-
>  hw/char/nrf51_uart.c      | 2 +-
>  hw/char/serial.c          | 2 +-
>  hw/char/virtio-console.c  | 2 +-
>  hw/usb/redirect.c         | 2 +-
>  hw/virtio/vhost-user.c    | 2 +-
>  monitor/monitor.c         | 2 +-
>  net/vhost-user.c          | 4 ++--
>  12 files changed, 19 insertions(+), 13 deletions(-)
> 
> diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
> index a553843364..867ef1b3b2 100644
> --- a/include/chardev/char-fe.h
> +++ b/include/chardev/char-fe.h
> @@ -174,6 +174,9 @@ void qemu_chr_fe_set_open(CharBackend *be, int fe_open);
>  void qemu_chr_fe_printf(CharBackend *be, const char *fmt, ...)
>      GCC_FMT_ATTR(2, 3);
>  
> +
> +typedef gboolean (*FEWatchFunc)(void *do_not_use, GIOCondition condition, void *data);
> +
>  /**
>   * qemu_chr_fe_add_watch:
>   * @cond: the condition to poll for
> @@ -188,10 +191,13 @@ void qemu_chr_fe_printf(CharBackend *be, const char *fmt, ...)
>   * Note that you are responsible to update the front-end sources if
>   * you are switching the main context with qemu_chr_fe_set_handlers().
>   *
> + * Warning: DO NOT use the first callback argument (it may be either
> + * a GIOChannel or a QIOChannel, depending on the underlying chardev)
> + *
>   * Returns: the source tag
>   */
>  guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition cond,
> -                            GIOFunc func, void *user_data);
> +                            FEWatchFunc func, void *user_data);
>  
>  /**
>   * qemu_chr_fe_write:
> diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> index 474715c5a9..7789f7be9c 100644
> --- a/chardev/char-fe.c
> +++ b/chardev/char-fe.c
> @@ -354,7 +354,7 @@ void qemu_chr_fe_set_open(CharBackend *be, int fe_open)
>  }
>  
>  guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition cond,
> -                            GIOFunc func, void *user_data)
> +                            FEWatchFunc func, void *user_data)
>  {
>      Chardev *s = be->chr;
>      GSource *src;
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index ceb677bc5a..8ee6f74b8c 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -288,7 +288,7 @@ static void uart_write_rx_fifo(void *opaque, const uint8_t *buf, int size)
>      uart_update_status(s);
>  }
>  
> -static gboolean cadence_uart_xmit(GIOChannel *chan, GIOCondition cond,
> +static gboolean cadence_uart_xmit(GIOChannel *do_not_use, GIOCondition cond,
>                                    void *opaque)

Why is this (and the next 3) left as GIOCondition, when you change others
later on to be void ?

>  {
>      CadenceUARTState *s = opaque;
> diff --git a/hw/char/cmsdk-apb-uart.c b/hw/char/cmsdk-apb-uart.c
> index ba2cbbee3d..b07a9dee4f 100644
> --- a/hw/char/cmsdk-apb-uart.c
> +++ b/hw/char/cmsdk-apb-uart.c
> @@ -191,7 +191,7 @@ static uint64_t uart_read(void *opaque, hwaddr offset, unsigned size)
>  /* Try to send tx data, and arrange to be called back later if
>   * we can't (ie the char backend is busy/blocking).
>   */
> -static gboolean uart_transmit(GIOChannel *chan, GIOCondition cond, void *opaque)
> +static gboolean uart_transmit(GIOChannel *do_not_use, GIOCondition cond, void *opaque)
>  {
>      CMSDKAPBUART *s = CMSDK_APB_UART(opaque);
>      int ret;
> diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
> index 6b0c9330bf..e493ea08c0 100644
> --- a/hw/char/ibex_uart.c
> +++ b/hw/char/ibex_uart.c
> @@ -135,7 +135,7 @@ static void ibex_uart_receive(void *opaque, const uint8_t *buf, int size)
>      ibex_uart_update_irqs(s);
>  }
>  
> -static gboolean ibex_uart_xmit(GIOChannel *chan, GIOCondition cond,
> +static gboolean ibex_uart_xmit(GIOChannel *do_not_use, GIOCondition cond,
>                                 void *opaque)
>  {
>      IbexUartState *s = opaque;
> diff --git a/hw/char/nrf51_uart.c b/hw/char/nrf51_uart.c
> index 045ca5fa40..0b89b0eae4 100644
> --- a/hw/char/nrf51_uart.c
> +++ b/hw/char/nrf51_uart.c
> @@ -75,7 +75,7 @@ static uint64_t uart_read(void *opaque, hwaddr addr, unsigned int size)
>      return r;
>  }
>  
> -static gboolean uart_transmit(GIOChannel *chan, GIOCondition cond, void *opaque)
> +static gboolean uart_transmit(GIOChannel *do_not_use, GIOCondition cond, void *opaque)
>  {
>      NRF51UARTState *s = NRF51_UART(opaque);
>      int r;
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index bc2e322970..7061aacbce 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -220,7 +220,7 @@ static void serial_update_msl(SerialState *s)
>      }
>  }
>  
> -static gboolean serial_watch_cb(GIOChannel *chan, GIOCondition cond,
> +static gboolean serial_watch_cb(void *do_not_use, GIOCondition cond,
>                                  void *opaque)
>  {
>      SerialState *s = opaque;
> diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
> index 6b132caa29..dd5a02e339 100644
> --- a/hw/char/virtio-console.c
> +++ b/hw/char/virtio-console.c
> @@ -38,7 +38,7 @@ struct VirtConsole {
>   * Callback function that's called from chardevs when backend becomes
>   * writable.
>   */
> -static gboolean chr_write_unblocked(GIOChannel *chan, GIOCondition cond,
> +static gboolean chr_write_unblocked(void *do_not_use, GIOCondition cond,
>                                      void *opaque)
>  {
>      VirtConsole *vcon = opaque;
> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> index 1ec909a63a..5f0ef9cb3b 100644
> --- a/hw/usb/redirect.c
> +++ b/hw/usb/redirect.c
> @@ -270,7 +270,7 @@ static int usbredir_read(void *priv, uint8_t *data, int count)
>      return count;
>  }
>  
> -static gboolean usbredir_write_unblocked(GIOChannel *chan, GIOCondition cond,
> +static gboolean usbredir_write_unblocked(void *do_not_use, GIOCondition cond,
>                                           void *opaque)
>  {
>      USBRedirDevice *dev = opaque;
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 29ea2b4fce..aec6cc1990 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -303,7 +303,7 @@ struct vhost_user_read_cb_data {
>      int ret;
>  };
>  
> -static gboolean vhost_user_read_cb(GIOChannel *source, GIOCondition condition,
> +static gboolean vhost_user_read_cb(void *do_not_use, GIOCondition condition,
>                                     gpointer opaque)
>  {
>      struct vhost_user_read_cb_data *data = opaque;
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index b90c0f4051..46a171bca6 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -156,7 +156,7 @@ static inline bool monitor_is_hmp_non_interactive(const Monitor *mon)
>  
>  static void monitor_flush_locked(Monitor *mon);
>  
> -static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
> +static gboolean monitor_unblocked(void *do_not_use, GIOCondition cond,
>                                    void *opaque)
>  {
>      Monitor *mon = opaque;
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index ffbd94d944..6adfcd623a 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -208,8 +208,8 @@ static NetClientInfo net_vhost_user_info = {
>          .set_vnet_le = vhost_user_set_vnet_endianness,
>  };
>  
> -static gboolean net_vhost_user_watch(GIOChannel *chan, GIOCondition cond,
> -                                           void *opaque)
> +static gboolean net_vhost_user_watch(void *do_not_use, GIOCondition cond,
> +                                     void *opaque)
>  {
>      NetVhostUserState *s = opaque;
>  
> -- 
> 2.32.0.264.g75ae10bc75
> 

Regards,
Daniel
Marc-André Lureau Aug. 5, 2021, 10:39 a.m. UTC | #2
Hi

On Thu, Aug 5, 2021 at 2:37 PM Daniel P. Berrangé <berrange@redhat.com>
wrote:

> On Wed, Aug 04, 2021 at 07:48:42PM +0400, marcandre.lureau@redhat.com
> wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Since commit 9894dc0cdcc397ee5b26370bc53da6d360a363c2 "char: convert
> > from GIOChannel to QIOChannel", the first argument to the watch callback
> > can actually be a QIOChannel, which is not a GIOChannel (but a QEMU
> > Object).
> >
> > Even though we never used that pointer, change the callback type to warn
> > the users. Possibly a better fix later, we may want to store the
> > callback and call it from intermediary functions.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  include/chardev/char-fe.h | 8 +++++++-
> >  chardev/char-fe.c         | 2 +-
> >  hw/char/cadence_uart.c    | 2 +-
> >  hw/char/cmsdk-apb-uart.c  | 2 +-
> >  hw/char/ibex_uart.c       | 2 +-
> >  hw/char/nrf51_uart.c      | 2 +-
> >  hw/char/serial.c          | 2 +-
> >  hw/char/virtio-console.c  | 2 +-
> >  hw/usb/redirect.c         | 2 +-
> >  hw/virtio/vhost-user.c    | 2 +-
> >  monitor/monitor.c         | 2 +-
> >  net/vhost-user.c          | 4 ++--
> >  12 files changed, 19 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
> > index a553843364..867ef1b3b2 100644
> > --- a/include/chardev/char-fe.h
> > +++ b/include/chardev/char-fe.h
> > @@ -174,6 +174,9 @@ void qemu_chr_fe_set_open(CharBackend *be, int
> fe_open);
> >  void qemu_chr_fe_printf(CharBackend *be, const char *fmt, ...)
> >      GCC_FMT_ATTR(2, 3);
> >
> > +
> > +typedef gboolean (*FEWatchFunc)(void *do_not_use, GIOCondition
> condition, void *data);
> > +
> >  /**
> >   * qemu_chr_fe_add_watch:
> >   * @cond: the condition to poll for
> > @@ -188,10 +191,13 @@ void qemu_chr_fe_printf(CharBackend *be, const
> char *fmt, ...)
> >   * Note that you are responsible to update the front-end sources if
> >   * you are switching the main context with qemu_chr_fe_set_handlers().
> >   *
> > + * Warning: DO NOT use the first callback argument (it may be either
> > + * a GIOChannel or a QIOChannel, depending on the underlying chardev)
> > + *
> >   * Returns: the source tag
> >   */
> >  guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition cond,
> > -                            GIOFunc func, void *user_data);
> > +                            FEWatchFunc func, void *user_data);
> >
> >  /**
> >   * qemu_chr_fe_write:
> > diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> > index 474715c5a9..7789f7be9c 100644
> > --- a/chardev/char-fe.c
> > +++ b/chardev/char-fe.c
> > @@ -354,7 +354,7 @@ void qemu_chr_fe_set_open(CharBackend *be, int
> fe_open)
> >  }
> >
> >  guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition cond,
> > -                            GIOFunc func, void *user_data)
> > +                            FEWatchFunc func, void *user_data)
> >  {
> >      Chardev *s = be->chr;
> >      GSource *src;
> > diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> > index ceb677bc5a..8ee6f74b8c 100644
> > --- a/hw/char/cadence_uart.c
> > +++ b/hw/char/cadence_uart.c
> > @@ -288,7 +288,7 @@ static void uart_write_rx_fifo(void *opaque, const
> uint8_t *buf, int size)
> >      uart_update_status(s);
> >  }
> >
> > -static gboolean cadence_uart_xmit(GIOChannel *chan, GIOCondition cond,
> > +static gboolean cadence_uart_xmit(GIOChannel *do_not_use, GIOCondition
> cond,
> >                                    void *opaque)
>
> Why is this (and the next 3) left as GIOCondition, when you change others
> later on to be void ?
>

Good catch. It's a leftover. It is fixed in my branch
https://gitlab.com/marcandre.lureau/qemu/-/tree/chardev

thanks


> >  {
> >      CadenceUARTState *s = opaque;
> > diff --git a/hw/char/cmsdk-apb-uart.c b/hw/char/cmsdk-apb-uart.c
> > index ba2cbbee3d..b07a9dee4f 100644
> > --- a/hw/char/cmsdk-apb-uart.c
> > +++ b/hw/char/cmsdk-apb-uart.c
> > @@ -191,7 +191,7 @@ static uint64_t uart_read(void *opaque, hwaddr
> offset, unsigned size)
> >  /* Try to send tx data, and arrange to be called back later if
> >   * we can't (ie the char backend is busy/blocking).
> >   */
> > -static gboolean uart_transmit(GIOChannel *chan, GIOCondition cond, void
> *opaque)
> > +static gboolean uart_transmit(GIOChannel *do_not_use, GIOCondition
> cond, void *opaque)
> >  {
> >      CMSDKAPBUART *s = CMSDK_APB_UART(opaque);
> >      int ret;
> > diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
> > index 6b0c9330bf..e493ea08c0 100644
> > --- a/hw/char/ibex_uart.c
> > +++ b/hw/char/ibex_uart.c
> > @@ -135,7 +135,7 @@ static void ibex_uart_receive(void *opaque, const
> uint8_t *buf, int size)
> >      ibex_uart_update_irqs(s);
> >  }
> >
> > -static gboolean ibex_uart_xmit(GIOChannel *chan, GIOCondition cond,
> > +static gboolean ibex_uart_xmit(GIOChannel *do_not_use, GIOCondition
> cond,
> >                                 void *opaque)
> >  {
> >      IbexUartState *s = opaque;
> > diff --git a/hw/char/nrf51_uart.c b/hw/char/nrf51_uart.c
> > index 045ca5fa40..0b89b0eae4 100644
> > --- a/hw/char/nrf51_uart.c
> > +++ b/hw/char/nrf51_uart.c
> > @@ -75,7 +75,7 @@ static uint64_t uart_read(void *opaque, hwaddr addr,
> unsigned int size)
> >      return r;
> >  }
> >
> > -static gboolean uart_transmit(GIOChannel *chan, GIOCondition cond, void
> *opaque)
> > +static gboolean uart_transmit(GIOChannel *do_not_use, GIOCondition
> cond, void *opaque)
> >  {
> >      NRF51UARTState *s = NRF51_UART(opaque);
> >      int r;
> > diff --git a/hw/char/serial.c b/hw/char/serial.c
> > index bc2e322970..7061aacbce 100644
> > --- a/hw/char/serial.c
> > +++ b/hw/char/serial.c
> > @@ -220,7 +220,7 @@ static void serial_update_msl(SerialState *s)
> >      }
> >  }
> >
> > -static gboolean serial_watch_cb(GIOChannel *chan, GIOCondition cond,
> > +static gboolean serial_watch_cb(void *do_not_use, GIOCondition cond,
> >                                  void *opaque)
> >  {
> >      SerialState *s = opaque;
> > diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
> > index 6b132caa29..dd5a02e339 100644
> > --- a/hw/char/virtio-console.c
> > +++ b/hw/char/virtio-console.c
> > @@ -38,7 +38,7 @@ struct VirtConsole {
> >   * Callback function that's called from chardevs when backend becomes
> >   * writable.
> >   */
> > -static gboolean chr_write_unblocked(GIOChannel *chan, GIOCondition cond,
> > +static gboolean chr_write_unblocked(void *do_not_use, GIOCondition cond,
> >                                      void *opaque)
> >  {
> >      VirtConsole *vcon = opaque;
> > diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> > index 1ec909a63a..5f0ef9cb3b 100644
> > --- a/hw/usb/redirect.c
> > +++ b/hw/usb/redirect.c
> > @@ -270,7 +270,7 @@ static int usbredir_read(void *priv, uint8_t *data,
> int count)
> >      return count;
> >  }
> >
> > -static gboolean usbredir_write_unblocked(GIOChannel *chan, GIOCondition
> cond,
> > +static gboolean usbredir_write_unblocked(void *do_not_use, GIOCondition
> cond,
> >                                           void *opaque)
> >  {
> >      USBRedirDevice *dev = opaque;
> > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> > index 29ea2b4fce..aec6cc1990 100644
> > --- a/hw/virtio/vhost-user.c
> > +++ b/hw/virtio/vhost-user.c
> > @@ -303,7 +303,7 @@ struct vhost_user_read_cb_data {
> >      int ret;
> >  };
> >
> > -static gboolean vhost_user_read_cb(GIOChannel *source, GIOCondition
> condition,
> > +static gboolean vhost_user_read_cb(void *do_not_use, GIOCondition
> condition,
> >                                     gpointer opaque)
> >  {
> >      struct vhost_user_read_cb_data *data = opaque;
> > diff --git a/monitor/monitor.c b/monitor/monitor.c
> > index b90c0f4051..46a171bca6 100644
> > --- a/monitor/monitor.c
> > +++ b/monitor/monitor.c
> > @@ -156,7 +156,7 @@ static inline bool
> monitor_is_hmp_non_interactive(const Monitor *mon)
> >
> >  static void monitor_flush_locked(Monitor *mon);
> >
> > -static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
> > +static gboolean monitor_unblocked(void *do_not_use, GIOCondition cond,
> >                                    void *opaque)
> >  {
> >      Monitor *mon = opaque;
> > diff --git a/net/vhost-user.c b/net/vhost-user.c
> > index ffbd94d944..6adfcd623a 100644
> > --- a/net/vhost-user.c
> > +++ b/net/vhost-user.c
> > @@ -208,8 +208,8 @@ static NetClientInfo net_vhost_user_info = {
> >          .set_vnet_le = vhost_user_set_vnet_endianness,
> >  };
> >
> > -static gboolean net_vhost_user_watch(GIOChannel *chan, GIOCondition
> cond,
> > -                                           void *opaque)
> > +static gboolean net_vhost_user_watch(void *do_not_use, GIOCondition
> cond,
> > +                                     void *opaque)
> >  {
> >      NetVhostUserState *s = opaque;
> >
> > --
> > 2.32.0.264.g75ae10bc75
> >
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|
>
>
>
Daniel P. Berrangé Aug. 5, 2021, 10:43 a.m. UTC | #3
On Thu, Aug 05, 2021 at 02:39:28PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Aug 5, 2021 at 2:37 PM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> 
> > On Wed, Aug 04, 2021 at 07:48:42PM +0400, marcandre.lureau@redhat.com
> > wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > Since commit 9894dc0cdcc397ee5b26370bc53da6d360a363c2 "char: convert
> > > from GIOChannel to QIOChannel", the first argument to the watch callback
> > > can actually be a QIOChannel, which is not a GIOChannel (but a QEMU
> > > Object).
> > >
> > > Even though we never used that pointer, change the callback type to warn
> > > the users. Possibly a better fix later, we may want to store the
> > > callback and call it from intermediary functions.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  include/chardev/char-fe.h | 8 +++++++-
> > >  chardev/char-fe.c         | 2 +-
> > >  hw/char/cadence_uart.c    | 2 +-
> > >  hw/char/cmsdk-apb-uart.c  | 2 +-
> > >  hw/char/ibex_uart.c       | 2 +-
> > >  hw/char/nrf51_uart.c      | 2 +-
> > >  hw/char/serial.c          | 2 +-
> > >  hw/char/virtio-console.c  | 2 +-
> > >  hw/usb/redirect.c         | 2 +-
> > >  hw/virtio/vhost-user.c    | 2 +-
> > >  monitor/monitor.c         | 2 +-
> > >  net/vhost-user.c          | 4 ++--
> > >  12 files changed, 19 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
> > > index a553843364..867ef1b3b2 100644
> > > --- a/include/chardev/char-fe.h
> > > +++ b/include/chardev/char-fe.h
> > > @@ -174,6 +174,9 @@ void qemu_chr_fe_set_open(CharBackend *be, int
> > fe_open);
> > >  void qemu_chr_fe_printf(CharBackend *be, const char *fmt, ...)
> > >      GCC_FMT_ATTR(2, 3);
> > >
> > > +
> > > +typedef gboolean (*FEWatchFunc)(void *do_not_use, GIOCondition
> > condition, void *data);
> > > +
> > >  /**
> > >   * qemu_chr_fe_add_watch:
> > >   * @cond: the condition to poll for
> > > @@ -188,10 +191,13 @@ void qemu_chr_fe_printf(CharBackend *be, const
> > char *fmt, ...)
> > >   * Note that you are responsible to update the front-end sources if
> > >   * you are switching the main context with qemu_chr_fe_set_handlers().
> > >   *
> > > + * Warning: DO NOT use the first callback argument (it may be either
> > > + * a GIOChannel or a QIOChannel, depending on the underlying chardev)
> > > + *
> > >   * Returns: the source tag
> > >   */
> > >  guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition cond,
> > > -                            GIOFunc func, void *user_data);
> > > +                            FEWatchFunc func, void *user_data);
> > >
> > >  /**
> > >   * qemu_chr_fe_write:
> > > diff --git a/chardev/char-fe.c b/chardev/char-fe.c
> > > index 474715c5a9..7789f7be9c 100644
> > > --- a/chardev/char-fe.c
> > > +++ b/chardev/char-fe.c
> > > @@ -354,7 +354,7 @@ void qemu_chr_fe_set_open(CharBackend *be, int
> > fe_open)
> > >  }
> > >
> > >  guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition cond,
> > > -                            GIOFunc func, void *user_data)
> > > +                            FEWatchFunc func, void *user_data)
> > >  {
> > >      Chardev *s = be->chr;
> > >      GSource *src;
> > > diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> > > index ceb677bc5a..8ee6f74b8c 100644
> > > --- a/hw/char/cadence_uart.c
> > > +++ b/hw/char/cadence_uart.c
> > > @@ -288,7 +288,7 @@ static void uart_write_rx_fifo(void *opaque, const
> > uint8_t *buf, int size)
> > >      uart_update_status(s);
> > >  }
> > >
> > > -static gboolean cadence_uart_xmit(GIOChannel *chan, GIOCondition cond,
> > > +static gboolean cadence_uart_xmit(GIOChannel *do_not_use, GIOCondition
> > cond,
> > >                                    void *opaque)
> >
> > Why is this (and the next 3) left as GIOCondition, when you change others
> > later on to be void ?
> >
> 
> Good catch. It's a leftover. It is fixed in my branch
> https://gitlab.com/marcandre.lureau/qemu/-/tree/chardev

Ok, with that

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>



Regards,
Daniel
diff mbox series

Patch

diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index a553843364..867ef1b3b2 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -174,6 +174,9 @@  void qemu_chr_fe_set_open(CharBackend *be, int fe_open);
 void qemu_chr_fe_printf(CharBackend *be, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
 
+
+typedef gboolean (*FEWatchFunc)(void *do_not_use, GIOCondition condition, void *data);
+
 /**
  * qemu_chr_fe_add_watch:
  * @cond: the condition to poll for
@@ -188,10 +191,13 @@  void qemu_chr_fe_printf(CharBackend *be, const char *fmt, ...)
  * Note that you are responsible to update the front-end sources if
  * you are switching the main context with qemu_chr_fe_set_handlers().
  *
+ * Warning: DO NOT use the first callback argument (it may be either
+ * a GIOChannel or a QIOChannel, depending on the underlying chardev)
+ *
  * Returns: the source tag
  */
 guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition cond,
-                            GIOFunc func, void *user_data);
+                            FEWatchFunc func, void *user_data);
 
 /**
  * qemu_chr_fe_write:
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 474715c5a9..7789f7be9c 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -354,7 +354,7 @@  void qemu_chr_fe_set_open(CharBackend *be, int fe_open)
 }
 
 guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition cond,
-                            GIOFunc func, void *user_data)
+                            FEWatchFunc func, void *user_data)
 {
     Chardev *s = be->chr;
     GSource *src;
diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index ceb677bc5a..8ee6f74b8c 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -288,7 +288,7 @@  static void uart_write_rx_fifo(void *opaque, const uint8_t *buf, int size)
     uart_update_status(s);
 }
 
-static gboolean cadence_uart_xmit(GIOChannel *chan, GIOCondition cond,
+static gboolean cadence_uart_xmit(GIOChannel *do_not_use, GIOCondition cond,
                                   void *opaque)
 {
     CadenceUARTState *s = opaque;
diff --git a/hw/char/cmsdk-apb-uart.c b/hw/char/cmsdk-apb-uart.c
index ba2cbbee3d..b07a9dee4f 100644
--- a/hw/char/cmsdk-apb-uart.c
+++ b/hw/char/cmsdk-apb-uart.c
@@ -191,7 +191,7 @@  static uint64_t uart_read(void *opaque, hwaddr offset, unsigned size)
 /* Try to send tx data, and arrange to be called back later if
  * we can't (ie the char backend is busy/blocking).
  */
-static gboolean uart_transmit(GIOChannel *chan, GIOCondition cond, void *opaque)
+static gboolean uart_transmit(GIOChannel *do_not_use, GIOCondition cond, void *opaque)
 {
     CMSDKAPBUART *s = CMSDK_APB_UART(opaque);
     int ret;
diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
index 6b0c9330bf..e493ea08c0 100644
--- a/hw/char/ibex_uart.c
+++ b/hw/char/ibex_uart.c
@@ -135,7 +135,7 @@  static void ibex_uart_receive(void *opaque, const uint8_t *buf, int size)
     ibex_uart_update_irqs(s);
 }
 
-static gboolean ibex_uart_xmit(GIOChannel *chan, GIOCondition cond,
+static gboolean ibex_uart_xmit(GIOChannel *do_not_use, GIOCondition cond,
                                void *opaque)
 {
     IbexUartState *s = opaque;
diff --git a/hw/char/nrf51_uart.c b/hw/char/nrf51_uart.c
index 045ca5fa40..0b89b0eae4 100644
--- a/hw/char/nrf51_uart.c
+++ b/hw/char/nrf51_uart.c
@@ -75,7 +75,7 @@  static uint64_t uart_read(void *opaque, hwaddr addr, unsigned int size)
     return r;
 }
 
-static gboolean uart_transmit(GIOChannel *chan, GIOCondition cond, void *opaque)
+static gboolean uart_transmit(GIOChannel *do_not_use, GIOCondition cond, void *opaque)
 {
     NRF51UARTState *s = NRF51_UART(opaque);
     int r;
diff --git a/hw/char/serial.c b/hw/char/serial.c
index bc2e322970..7061aacbce 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -220,7 +220,7 @@  static void serial_update_msl(SerialState *s)
     }
 }
 
-static gboolean serial_watch_cb(GIOChannel *chan, GIOCondition cond,
+static gboolean serial_watch_cb(void *do_not_use, GIOCondition cond,
                                 void *opaque)
 {
     SerialState *s = opaque;
diff --git a/hw/char/virtio-console.c b/hw/char/virtio-console.c
index 6b132caa29..dd5a02e339 100644
--- a/hw/char/virtio-console.c
+++ b/hw/char/virtio-console.c
@@ -38,7 +38,7 @@  struct VirtConsole {
  * Callback function that's called from chardevs when backend becomes
  * writable.
  */
-static gboolean chr_write_unblocked(GIOChannel *chan, GIOCondition cond,
+static gboolean chr_write_unblocked(void *do_not_use, GIOCondition cond,
                                     void *opaque)
 {
     VirtConsole *vcon = opaque;
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 1ec909a63a..5f0ef9cb3b 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -270,7 +270,7 @@  static int usbredir_read(void *priv, uint8_t *data, int count)
     return count;
 }
 
-static gboolean usbredir_write_unblocked(GIOChannel *chan, GIOCondition cond,
+static gboolean usbredir_write_unblocked(void *do_not_use, GIOCondition cond,
                                          void *opaque)
 {
     USBRedirDevice *dev = opaque;
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 29ea2b4fce..aec6cc1990 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -303,7 +303,7 @@  struct vhost_user_read_cb_data {
     int ret;
 };
 
-static gboolean vhost_user_read_cb(GIOChannel *source, GIOCondition condition,
+static gboolean vhost_user_read_cb(void *do_not_use, GIOCondition condition,
                                    gpointer opaque)
 {
     struct vhost_user_read_cb_data *data = opaque;
diff --git a/monitor/monitor.c b/monitor/monitor.c
index b90c0f4051..46a171bca6 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -156,7 +156,7 @@  static inline bool monitor_is_hmp_non_interactive(const Monitor *mon)
 
 static void monitor_flush_locked(Monitor *mon);
 
-static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
+static gboolean monitor_unblocked(void *do_not_use, GIOCondition cond,
                                   void *opaque)
 {
     Monitor *mon = opaque;
diff --git a/net/vhost-user.c b/net/vhost-user.c
index ffbd94d944..6adfcd623a 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -208,8 +208,8 @@  static NetClientInfo net_vhost_user_info = {
         .set_vnet_le = vhost_user_set_vnet_endianness,
 };
 
-static gboolean net_vhost_user_watch(GIOChannel *chan, GIOCondition cond,
-                                           void *opaque)
+static gboolean net_vhost_user_watch(void *do_not_use, GIOCondition cond,
+                                     void *opaque)
 {
     NetVhostUserState *s = opaque;