[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 05/11] chardev: mark explicitly first argument as poisoned
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v2 05/11] chardev: mark explicitly first argument as poisoned |
Date: |
Thu, 5 Aug 2021 11:43:26 +0100 |
User-agent: |
Mutt/2.0.7 (2021-05-04) |
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
--
|: 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 :|
- [PATCH v2 00/11] chardev related fixes, marcandre . lureau, 2021/08/04
- [PATCH v2 01/11] util: fix abstract socket path copy, marcandre . lureau, 2021/08/04
- [PATCH v2 03/11] chardev: remove needless class method, marcandre . lureau, 2021/08/04
- [PATCH v2 07/11] chardev: fix qemu_chr_open_fd() being called with fd=-1, marcandre . lureau, 2021/08/04
- [PATCH v2 06/11] chardev: fix fd_chr_add_watch() when in != out, marcandre . lureau, 2021/08/04
- [PATCH v2 08/11] chardev: fix qemu_chr_open_fd() with fd_in==fd_out, marcandre . lureau, 2021/08/04
- [PATCH v2 04/11] chardev: add some comments about the class methods, marcandre . lureau, 2021/08/04
- [PATCH v2 02/11] chardev/socket: print a more correct command-line address, marcandre . lureau, 2021/08/04