Patchwork [1/5] char: Add a QemuChrHandlers struct to initialise chardev handlers

login
register
mail settings
Submitter Amit Shah
Date Jan. 11, 2011, 11:10 a.m.
Message ID <edcae4488c7a4e27a9065ebe01044f215974b5ce.1294743490.git.amit.shah@redhat.com>
Download mbox | patch
Permalink /patch/78350/
State New
Headers show

Comments

Amit Shah - Jan. 11, 2011, 11:10 a.m.
Instead of passing each handler in the qemu_add_handlers() function,
create a struct of handlers that can be passed to the function instead.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 gdbstub.c            |    9 +++++++--
 hw/debugcon.c        |    2 +-
 hw/escc.c            |    9 +++++++--
 hw/etraxfs_ser.c     |   13 +++++++++----
 hw/ivshmem.c         |   28 ++++++++++++++++++++++------
 hw/mcf_uart.c        |    9 +++++++--
 hw/pl011.c           |    9 +++++++--
 hw/pxa2xx.c          |   13 +++++++++----
 hw/serial.c          |    9 +++++++--
 hw/sh_serial.c       |   12 +++++++++---
 hw/syborg_serial.c   |    9 +++++++--
 hw/usb-serial.c      |    9 +++++++--
 hw/virtio-console.c  |    9 +++++++--
 hw/xen_console.c     |   16 +++++++++++-----
 hw/xilinx_uartlite.c |   11 +++++++++--
 monitor.c            |   19 +++++++++++++++----
 net/slirp.c          |    8 ++++++--
 qemu-char.c          |   27 ++++++++++++++++++---------
 qemu-char.h          |   12 ++++++++----
 19 files changed, 173 insertions(+), 60 deletions(-)
Gerd Hoffmann - Jan. 11, 2011, 2:17 p.m.
On 01/11/11 12:10, Amit Shah wrote:
> Instead of passing each handler in the qemu_add_handlers() function,
> create a struct of handlers that can be passed to the function instead.

Nice cleanup.

Acked-by: Gerd Hoffmann <kraxel@redhat.com>

cheers,
   Gerd
Blue Swirl - Jan. 11, 2011, 5:13 p.m.
On Tue, Jan 11, 2011 at 11:10 AM, Amit Shah <amit.shah@redhat.com> wrote:
> Instead of passing each handler in the qemu_add_handlers() function,
> create a struct of handlers that can be passed to the function instead.
>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>  gdbstub.c            |    9 +++++++--
>  hw/debugcon.c        |    2 +-
>  hw/escc.c            |    9 +++++++--
>  hw/etraxfs_ser.c     |   13 +++++++++----
>  hw/ivshmem.c         |   28 ++++++++++++++++++++++------
>  hw/mcf_uart.c        |    9 +++++++--
>  hw/pl011.c           |    9 +++++++--
>  hw/pxa2xx.c          |   13 +++++++++----
>  hw/serial.c          |    9 +++++++--
>  hw/sh_serial.c       |   12 +++++++++---
>  hw/syborg_serial.c   |    9 +++++++--
>  hw/usb-serial.c      |    9 +++++++--
>  hw/virtio-console.c  |    9 +++++++--
>  hw/xen_console.c     |   16 +++++++++++-----
>  hw/xilinx_uartlite.c |   11 +++++++++--
>  monitor.c            |   19 +++++++++++++++----
>  net/slirp.c          |    8 ++++++--
>  qemu-char.c          |   27 ++++++++++++++++++---------
>  qemu-char.h          |   12 ++++++++----
>  19 files changed, 173 insertions(+), 60 deletions(-)
>
> diff --git a/gdbstub.c b/gdbstub.c
> index 0aa081b..b5ba2ef 100644
> --- a/gdbstub.c
> +++ b/gdbstub.c
> @@ -2627,6 +2627,12 @@ static void gdb_sigterm_handler(int signal)
>  }
>  #endif
>
> +static QemuChrHandlers gdb_handlers = {
> +    .fd_can_read = gdb_chr_can_receive,
> +    .fd_read = gdb_chr_receive,
> +    .fd_event = gdb_chr_event,
> +};

These structures should be const.

> +
>  int gdbserver_start(const char *device)
>  {
>     GDBState *s;
> @@ -2656,8 +2662,7 @@ int gdbserver_start(const char *device)
>         if (!chr)
>             return -1;
>
> -        qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive,
> -                              gdb_chr_event, NULL);
> +        qemu_chr_add_handlers(chr, &gdb_handlers, NULL);
>     }
>
>     s = gdbserver_state;
> diff --git a/hw/debugcon.c b/hw/debugcon.c
> index 5ee6821..e79a595 100644
> --- a/hw/debugcon.c
> +++ b/hw/debugcon.c
> @@ -73,7 +73,7 @@ static void debugcon_init_core(DebugconState *s)
>         exit(1);
>     }
>
> -    qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, s);
> +    qemu_chr_add_handlers(s->chr, NULL, s);
>  }
>
>  static int debugcon_isa_initfn(ISADevice *dev)
> diff --git a/hw/escc.c b/hw/escc.c
> index ba60636..6be7b53 100644
> --- a/hw/escc.c
> +++ b/hw/escc.c
> @@ -894,6 +894,12 @@ void slavio_serial_ms_kbd_init(target_phys_addr_t base, qemu_irq irq,
>     sysbus_mmio_map(s, 0, base);
>  }
>
> +static QemuChrHandlers serial_handlers = {
> +    .fd_can_read = serial_can_receive,
> +    .fd_read = serial_receive1,
> +    .fd_event = serial_event,
> +};
> +
>  static int escc_init1(SysBusDevice *dev)
>  {
>     SerialState *s = FROM_SYSBUS(SerialState, dev);
> @@ -907,8 +913,7 @@ static int escc_init1(SysBusDevice *dev)
>         s->chn[i].chn = 1 - i;
>         s->chn[i].clock = s->frequency / 2;
>         if (s->chn[i].chr) {
> -            qemu_chr_add_handlers(s->chn[i].chr, serial_can_receive,
> -                                  serial_receive1, serial_event, &s->chn[i]);
> +            qemu_chr_add_handlers(s->chn[i].chr, &serial_handlers, &s->chn[i]);
>         }
>     }
>     s->chn[0].otherchn = &s->chn[1];
> diff --git a/hw/etraxfs_ser.c b/hw/etraxfs_ser.c
> index 2787ebd..4612c2b 100644
> --- a/hw/etraxfs_ser.c
> +++ b/hw/etraxfs_ser.c
> @@ -190,6 +190,12 @@ static void serial_event(void *opaque, int event)
>
>  }
>
> +static QemuChrHandlers serial_handlers = {
> +    .fd_can_read = serial_can_receive,
> +    .fd_read = serial_receive,
> +    .fd_event = serial_event,
> +};
> +
>  static int etraxfs_ser_init(SysBusDevice *dev)
>  {
>     struct etrax_serial *s = FROM_SYSBUS(typeof (*s), dev);
> @@ -204,10 +210,9 @@ static int etraxfs_ser_init(SysBusDevice *dev)
>                                       DEVICE_NATIVE_ENDIAN);
>     sysbus_init_mmio(dev, R_MAX * 4, ser_regs);
>     s->chr = qdev_init_chardev(&dev->qdev);
> -    if (s->chr)
> -        qemu_chr_add_handlers(s->chr,
> -                      serial_can_receive, serial_receive,
> -                      serial_event, s);
> +    if (s->chr) {
> +        qemu_chr_add_handlers(s->chr, &serial_handlers, s);
> +    }
>     return 0;
>  }
>
> diff --git a/hw/ivshmem.c b/hw/ivshmem.c
> index 7b19a81..2a47e98 100644
> --- a/hw/ivshmem.c
> +++ b/hw/ivshmem.c
> @@ -312,6 +312,18 @@ static void fake_irqfd(void *opaque, const uint8_t *buf, int size) {
>     msix_notify(pdev, entry->vector);
>  }
>
> +static QemuChrHandlers ivshmem_msi_handlers = {
> +    .fd_can_read = ivshmem_can_receive,
> +    .fd_read = fake_irqfd,
> +    .fd_event = ivshmem_event,
> +};
> +
> +static QemuChrHandlers ivshmem_nomsi_handlers = {
> +    .fd_can_read = ivshmem_can_receive,
> +    .fd_read = ivshmem_receive,
> +    .fd_event = ivshmem_event,
> +};
> +
>  static CharDriverState* create_eventfd_chr_device(void * opaque, int eventfd,
>                                                                     int vector)
>  {
> @@ -331,11 +343,10 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, int eventfd,
>         s->eventfd_table[vector].pdev = &s->dev;
>         s->eventfd_table[vector].vector = vector;
>
> -        qemu_chr_add_handlers(chr, ivshmem_can_receive, fake_irqfd,
> -                      ivshmem_event, &s->eventfd_table[vector]);
> +        qemu_chr_add_handlers(chr, &ivshmem_msi_handlers,
> +                              &s->eventfd_table[vector]);
>     } else {
> -        qemu_chr_add_handlers(chr, ivshmem_can_receive, ivshmem_receive,
> -                      ivshmem_event, s);
> +        qemu_chr_add_handlers(chr, &ivshmem_nomsi_handlers, s);
>     }
>
>     return chr;
> @@ -666,6 +677,12 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int version_id)
>     return 0;
>  }
>
> +static QemuChrHandlers ivshmem_handlers = {
> +    .fd_can_read = ivshmem_can_receive,
> +    .fd_read = ivshmem_read,
> +    .fd_event = ivshmem_event,
> +};
> +
>  static int pci_ivshmem_init(PCIDevice *dev)
>  {
>     IVShmemState *s = DO_UPCAST(IVShmemState, dev, dev);
> @@ -754,8 +771,7 @@ static int pci_ivshmem_init(PCIDevice *dev)
>
>         s->eventfd_chr = qemu_mallocz(s->vectors * sizeof(CharDriverState *));
>
> -        qemu_chr_add_handlers(s->server_chr, ivshmem_can_receive, ivshmem_read,
> -                     ivshmem_event, s);
> +        qemu_chr_add_handlers(s->server_chr, &ivshmem_handlers, s);
>     } else {
>         /* just map the file immediately, we're not using a server */
>         int fd;
> diff --git a/hw/mcf_uart.c b/hw/mcf_uart.c
> index db57096..f8ff526 100644
> --- a/hw/mcf_uart.c
> +++ b/hw/mcf_uart.c
> @@ -268,6 +268,12 @@ static void mcf_uart_receive(void *opaque, const uint8_t *buf, int size)
>     mcf_uart_push_byte(s, buf[0]);
>  }
>
> +static QemuChrHandlers mcf_uart_handlers = {
> +    .fd_can_read = mcf_uart_can_receive,
> +    .fd_read = mcf_uart_receive,
> +    .fd_event = mcf_uart_event,
> +};
> +
>  void *mcf_uart_init(qemu_irq irq, CharDriverState *chr)
>  {
>     mcf_uart_state *s;
> @@ -276,8 +282,7 @@ void *mcf_uart_init(qemu_irq irq, CharDriverState *chr)
>     s->chr = chr;
>     s->irq = irq;
>     if (chr) {
> -        qemu_chr_add_handlers(chr, mcf_uart_can_receive, mcf_uart_receive,
> -                              mcf_uart_event, s);
> +        qemu_chr_add_handlers(chr, &mcf_uart_handlers, s);
>     }
>     mcf_uart_reset(s);
>     return s;
> diff --git a/hw/pl011.c b/hw/pl011.c
> index 77f0dbf..107601a 100644
> --- a/hw/pl011.c
> +++ b/hw/pl011.c
> @@ -286,6 +286,12 @@ static int pl011_load(QEMUFile *f, void *opaque, int version_id)
>     return 0;
>  }
>
> +static QemuChrHandlers pl011_handlers = {
> +    .fd_can_read = pl011_can_receive,
> +    .fd_read = pl011_receive,
> +    .fd_event = pl011_event,
> +};
> +
>  static int pl011_init(SysBusDevice *dev, const unsigned char *id)
>  {
>     int iomemtype;
> @@ -304,8 +310,7 @@ static int pl011_init(SysBusDevice *dev, const unsigned char *id)
>     s->cr = 0x300;
>     s->flags = 0x90;
>     if (s->chr) {
> -        qemu_chr_add_handlers(s->chr, pl011_can_receive, pl011_receive,
> -                              pl011_event, s);
> +        qemu_chr_add_handlers(s->chr, &pl011_handlers, s);
>     }
>     register_savevm(&dev->qdev, "pl011_uart", -1, 1, pl011_save, pl011_load, s);
>     return 0;
> diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c
> index ab524a7..952732a 100644
> --- a/hw/pxa2xx.c
> +++ b/hw/pxa2xx.c
> @@ -1995,6 +1995,12 @@ static int pxa2xx_fir_load(QEMUFile *f, void *opaque, int version_id)
>     return 0;
>  }
>
> +static QemuChrHandlers pxa2xx_handlers = {
> +    .fd_can_read = pxa2xx_fir_is_empty,
> +    .fd_read = pxa2xx_fir_rx,
> +    .fd_event = pxa2xx_fir_event,
> +};
> +
>  static PXA2xxFIrState *pxa2xx_fir_init(target_phys_addr_t base,
>                 qemu_irq irq, PXA2xxDMAState *dma,
>                 CharDriverState *chr)
> @@ -2013,10 +2019,9 @@ static PXA2xxFIrState *pxa2xx_fir_init(target_phys_addr_t base,
>                     pxa2xx_fir_writefn, s, DEVICE_NATIVE_ENDIAN);
>     cpu_register_physical_memory(base, 0x1000, iomemtype);
>
> -    if (chr)
> -        qemu_chr_add_handlers(chr, pxa2xx_fir_is_empty,
> -                        pxa2xx_fir_rx, pxa2xx_fir_event, s);
> -
> +    if (chr) {
> +        qemu_chr_add_handlers(chr, &pxa2xx_handlers, s);
> +    }
>     register_savevm(NULL, "pxa2xx_fir", 0, 0, pxa2xx_fir_save,
>                     pxa2xx_fir_load, s);
>
> diff --git a/hw/serial.c b/hw/serial.c
> index 2c4af61..5ed4b9f 100644
> --- a/hw/serial.c
> +++ b/hw/serial.c
> @@ -727,6 +727,12 @@ static void serial_reset(void *opaque)
>     qemu_irq_lower(s->irq);
>  }
>
> +static QemuChrHandlers serial_handlers = {
> +    .fd_can_read = serial_can_receive1,
> +    .fd_read = serial_receive1,
> +    .fd_event = serial_event,
> +};
> +
>  static void serial_init_core(SerialState *s)
>  {
>     if (!s->chr) {
> @@ -741,8 +747,7 @@ static void serial_init_core(SerialState *s)
>
>     qemu_register_reset(serial_reset, s);
>
> -    qemu_chr_add_handlers(s->chr, serial_can_receive1, serial_receive1,
> -                          serial_event, s);
> +    qemu_chr_add_handlers(s->chr, &serial_handlers, s);
>  }
>
>  /* Change the main reference oscillator frequency. */
> diff --git a/hw/sh_serial.c b/hw/sh_serial.c
> index 1bdc0a5..69ab6e3 100644
> --- a/hw/sh_serial.c
> +++ b/hw/sh_serial.c
> @@ -363,6 +363,12 @@ static CPUWriteMemoryFunc * const sh_serial_writefn[] = {
>     &sh_serial_write,
>  };
>
> +static QemuChrHandlers sh_serial_handlers = {
> +    .fd_can_read = sh_serial_can_receive1,
> +    .fd_read = sh_serial_receive1,
> +    .fd_event = sh_serial_event,
> +};
> +
>  void sh_serial_init (target_phys_addr_t base, int feat,
>                     uint32_t freq, CharDriverState *chr,
>                     qemu_irq eri_source,
> @@ -402,9 +408,9 @@ void sh_serial_init (target_phys_addr_t base, int feat,
>
>     s->chr = chr;
>
> -    if (chr)
> -        qemu_chr_add_handlers(chr, sh_serial_can_receive1, sh_serial_receive1,
> -                             sh_serial_event, s);
> +    if (chr) {
> +        qemu_chr_add_handlers(chr, &sh_serial_handlers, s);
> +    }
>
>     s->eri = eri_source;
>     s->rxi = rxi_source;
> diff --git a/hw/syborg_serial.c b/hw/syborg_serial.c
> index 34ce076..1e6b60b 100644
> --- a/hw/syborg_serial.c
> +++ b/hw/syborg_serial.c
> @@ -315,6 +315,12 @@ static int syborg_serial_load(QEMUFile *f, void *opaque, int version_id)
>     return 0;
>  }
>
> +static QemuChrHandlers syborg_serial_handlers = {
> +    .fd_can_read = syborg_serial_can_receive,
> +    .fd_read = syborg_serial_receive,
> +    .fd_event = syborg_serial_event,
> +};
> +
>  static int syborg_serial_init(SysBusDevice *dev)
>  {
>     SyborgSerialState *s = FROM_SYSBUS(SyborgSerialState, dev);
> @@ -327,8 +333,7 @@ static int syborg_serial_init(SysBusDevice *dev)
>     sysbus_init_mmio(dev, 0x1000, iomemtype);
>     s->chr = qdev_init_chardev(&dev->qdev);
>     if (s->chr) {
> -        qemu_chr_add_handlers(s->chr, syborg_serial_can_receive,
> -                              syborg_serial_receive, syborg_serial_event, s);
> +        qemu_chr_add_handlers(s->chr, &syborg_serial_handlers, s);
>     }
>     if (s->fifo_size <= 0) {
>         fprintf(stderr, "syborg_serial: fifo too small\n");
> diff --git a/hw/usb-serial.c b/hw/usb-serial.c
> index c19580f..697cb89 100644
> --- a/hw/usb-serial.c
> +++ b/hw/usb-serial.c
> @@ -540,6 +540,12 @@ static void usb_serial_event(void *opaque, int event)
>     }
>  }
>
> +static QemuChrHandlers usb_serial_handlers = {
> +    .fd_can_read = usb_serial_can_read,
> +    .fd_read = usb_serial_read,
> +    .fd_event = usb_serial_event,
> +};
> +
>  static int usb_serial_initfn(USBDevice *dev)
>  {
>     USBSerialState *s = DO_UPCAST(USBSerialState, dev, dev);
> @@ -550,8 +556,7 @@ static int usb_serial_initfn(USBDevice *dev)
>         return -1;
>     }
>
> -    qemu_chr_add_handlers(s->cs, usb_serial_can_read, usb_serial_read,
> -                          usb_serial_event, s);
> +    qemu_chr_add_handlers(s->cs, &usb_serial_handlers, s);
>     usb_serial_handle_reset(dev);
>     return 0;
>  }
> diff --git a/hw/virtio-console.c b/hw/virtio-console.c
> index d0b9354..3bd2b79 100644
> --- a/hw/virtio-console.c
> +++ b/hw/virtio-console.c
> @@ -57,13 +57,18 @@ static void chr_event(void *opaque, int event)
>     }
>  }
>
> +static QemuChrHandlers chr_handlers = {
> +    .fd_can_read = chr_can_read,
> +    .fd_read = chr_read,
> +    .fd_event = chr_event,
> +};
> +
>  static int generic_port_init(VirtConsole *vcon, VirtIOSerialDevice *dev)
>  {
>     vcon->port.info = dev->info;
>
>     if (vcon->chr) {
> -        qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event,
> -                              vcon);
> +        qemu_chr_add_handlers(vcon->chr, &chr_handlers, vcon);
>         vcon->port.info->have_data = flush_buf;
>     }
>     return 0;
> diff --git a/hw/xen_console.c b/hw/xen_console.c
> index d2261f4..f490a5a 100644
> --- a/hw/xen_console.c
> +++ b/hw/xen_console.c
> @@ -202,6 +202,11 @@ static int con_init(struct XenDevice *xendev)
>     return 0;
>  }
>
> +static QemuChrHandlers xencons_handlers = {
> +    .fd_can_read = xencons_can_receive,
> +    .fd_read = xencons_receive,
> +};
> +
>  static int con_connect(struct XenDevice *xendev)
>  {
>     struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
> @@ -222,9 +227,9 @@ static int con_connect(struct XenDevice *xendev)
>        return -1;
>
>     xen_be_bind_evtchn(&con->xendev);
> -    if (con->chr)
> -        qemu_chr_add_handlers(con->chr, xencons_can_receive, xencons_receive,
> -                              NULL, con);
> +    if (con->chr) {
> +        qemu_chr_add_handlers(con->chr, &xencons_handlers, con);
> +    }
>
>     xen_be_printf(xendev, 1, "ring mfn %d, remote port %d, local port %d, limit %zd\n",
>                  con->ring_ref,
> @@ -238,8 +243,9 @@ static void con_disconnect(struct XenDevice *xendev)
>  {
>     struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
>
> -    if (con->chr)
> -        qemu_chr_add_handlers(con->chr, NULL, NULL, NULL, NULL);
> +    if (con->chr) {
> +        qemu_chr_add_handlers(con->chr, NULL, NULL);
> +    }
>     xen_be_unbind_evtchn(&con->xendev);
>
>     if (con->sring) {
> diff --git a/hw/xilinx_uartlite.c b/hw/xilinx_uartlite.c
> index 9b94e98..9ae84f7 100644
> --- a/hw/xilinx_uartlite.c
> +++ b/hw/xilinx_uartlite.c
> @@ -193,6 +193,12 @@ static void uart_event(void *opaque, int event)
>
>  }
>
> +static QemuChrHandlers uart_handlers = {
> +    .fd_can_read = uart_can_rx,
> +    .fd_read = uart_rx,
> +    .fd_event = uart_event,
> +};
> +
>  static int xilinx_uartlite_init(SysBusDevice *dev)
>  {
>     struct xlx_uartlite *s = FROM_SYSBUS(typeof (*s), dev);
> @@ -206,8 +212,9 @@ static int xilinx_uartlite_init(SysBusDevice *dev)
>     sysbus_init_mmio(dev, R_MAX * 4, uart_regs);
>
>     s->chr = qdev_init_chardev(&dev->qdev);
> -    if (s->chr)
> -        qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
> +    if (s->chr) {
> +        qemu_chr_add_handlers(s->chr, &uart_handlers, s);
> +    }
>     return 0;
>  }
>
> diff --git a/monitor.c b/monitor.c
> index f258000..0c16970 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -5110,6 +5110,18 @@ static void monitor_event(void *opaque, int event)
>  * End:
>  */
>
> +static QemuChrHandlers monitor_handlers = {
> +    .fd_can_read = monitor_can_read,
> +    .fd_read = monitor_read,
> +    .fd_event = monitor_event,
> +};
> +
> +static QemuChrHandlers monitor_control_handlers = {
> +    .fd_can_read = monitor_can_read,
> +    .fd_read = monitor_control_read,
> +    .fd_event = monitor_control_event,
> +};
> +
>  void monitor_init(CharDriverState *chr, int flags)
>  {
>     static int is_first_init = 1;
> @@ -5131,12 +5143,11 @@ void monitor_init(CharDriverState *chr, int flags)
>
>     if (monitor_ctrl_mode(mon)) {
>         mon->mc = qemu_mallocz(sizeof(MonitorControl));
> +
>         /* Control mode requires special handlers */
> -        qemu_chr_add_handlers(chr, monitor_can_read, monitor_control_read,
> -                              monitor_control_event, mon);
> +        qemu_chr_add_handlers(chr, &monitor_control_handlers, mon);
>     } else {
> -        qemu_chr_add_handlers(chr, monitor_can_read, monitor_read,
> -                              monitor_event, mon);
> +        qemu_chr_add_handlers(chr, &monitor_handlers, mon);
>     }
>
>     QLIST_INSERT_HEAD(&mon_list, mon, entry);
> diff --git a/net/slirp.c b/net/slirp.c
> index b41c60a..52e9a5c 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -577,6 +577,11 @@ static void guestfwd_read(void *opaque, const uint8_t *buf, int size)
>     slirp_socket_recv(fwd->slirp, fwd->server, fwd->port, buf, size);
>  }
>
> +static QemuChrHandlers guestfwd_handlers = {
> +    .fd_can_read = guestfwd_can_read,
> +    .fd_read = guestfwd_read,
> +};
> +
>  static int slirp_guestfwd(SlirpState *s, const char *config_str,
>                           int legacy_format)
>  {
> @@ -633,8 +638,7 @@ static int slirp_guestfwd(SlirpState *s, const char *config_str,
>     fwd->port = port;
>     fwd->slirp = s->slirp;
>
> -    qemu_chr_add_handlers(fwd->hd, guestfwd_can_read, guestfwd_read,
> -                          NULL, fwd);
> +    qemu_chr_add_handlers(fwd->hd, &guestfwd_handlers, fwd);
>     return 0;
>
>  fail_syntax:
> diff --git a/qemu-char.c b/qemu-char.c
> index edc9ad6..d9a1df3 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -190,15 +190,19 @@ void qemu_chr_send_event(CharDriverState *s, int event)
>         s->chr_send_event(s, event);
>  }
>
> +static QemuChrHandlers null_handlers = {
> +    /* All handlers are initialised to NULL */
> +};
> +
>  void qemu_chr_add_handlers(CharDriverState *s,
> -                           IOCanReadHandler *fd_can_read,
> -                           IOReadHandler *fd_read,
> -                           IOEventHandler *fd_event,
> -                           void *opaque)
> -{
> -    s->chr_can_read = fd_can_read;
> -    s->chr_read = fd_read;
> -    s->chr_event = fd_event;
> +                           QemuChrHandlers *handlers, void *opaque)
> +{

Here we could also check if (!s) and return if so. This would simplify
the callers a bit.
Amit Shah - Jan. 12, 2011, 6:07 a.m.
On (Tue) Jan 11 2011 [17:13:15], Blue Swirl wrote:
> > +static QemuChrHandlers gdb_handlers = {
> > +    .fd_can_read = gdb_chr_can_receive,
> > +    .fd_read = gdb_chr_receive,
> > +    .fd_event = gdb_chr_event,
> > +};
> 
> These structures should be const.

Hm, I had that but looks like it got lost in some rebase.  Added again.

> > @@ -190,15 +190,19 @@ void qemu_chr_send_event(CharDriverState *s, int event)
> >         s->chr_send_event(s, event);
> >  }
> >
> > +static QemuChrHandlers null_handlers = {
> > +    /* All handlers are initialised to NULL */
> > +};
> > +
> >  void qemu_chr_add_handlers(CharDriverState *s,
> > -                           IOCanReadHandler *fd_can_read,
> > -                           IOReadHandler *fd_read,
> > -                           IOEventHandler *fd_event,
> > -                           void *opaque)
> > -{
> > -    s->chr_can_read = fd_can_read;
> > -    s->chr_read = fd_read;
> > -    s->chr_event = fd_event;
> > +                           QemuChrHandlers *handlers, void *opaque)
> > +{
> 
> Here we could also check if (!s) and return if so. This would simplify
> the callers a bit.

Simplified in what way?

		Amit
Michael Roth - Jan. 12, 2011, 6:01 p.m.
On 01/12/2011 12:07 AM, Amit Shah wrote:
> On (Tue) Jan 11 2011 [17:13:15], Blue Swirl wrote:
>>> +static QemuChrHandlers gdb_handlers = {
>>> +    .fd_can_read = gdb_chr_can_receive,
>>> +    .fd_read = gdb_chr_receive,
>>> +    .fd_event = gdb_chr_event,
>>> +};
>>
>> These structures should be const.
>
> Hm, I had that but looks like it got lost in some rebase.  Added again.
>
>>> @@ -190,15 +190,19 @@ void qemu_chr_send_event(CharDriverState *s, int event)
>>>          s->chr_send_event(s, event);
>>>   }
>>>
>>> +static QemuChrHandlers null_handlers = {
>>> +    /* All handlers are initialised to NULL */
>>> +};
>>> +
>>>   void qemu_chr_add_handlers(CharDriverState *s,
>>> -                           IOCanReadHandler *fd_can_read,
>>> -                           IOReadHandler *fd_read,
>>> -                           IOEventHandler *fd_event,
>>> -                           void *opaque)
>>> -{
>>> -    s->chr_can_read = fd_can_read;
>>> -    s->chr_read = fd_read;
>>> -    s->chr_event = fd_event;
>>> +                           QemuChrHandlers *handlers, void *opaque)
>>> +{
>>
>> Here we could also check if (!s) and return if so. This would simplify
>> the callers a bit.
>
> Simplified in what way?

I assume for reducing the need to have to check s->chr != NULL everytime 
beforehand. It's safer and would save a lot on repetitive code as well.

>
> 		Amit
>
Blue Swirl - Jan. 12, 2011, 7:03 p.m.
On Wed, Jan 12, 2011 at 6:01 PM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> On 01/12/2011 12:07 AM, Amit Shah wrote:
>>
>> On (Tue) Jan 11 2011 [17:13:15], Blue Swirl wrote:
>>>>
>>>> +static QemuChrHandlers gdb_handlers = {
>>>> +    .fd_can_read = gdb_chr_can_receive,
>>>> +    .fd_read = gdb_chr_receive,
>>>> +    .fd_event = gdb_chr_event,
>>>> +};
>>>
>>> These structures should be const.
>>
>> Hm, I had that but looks like it got lost in some rebase.  Added again.
>>
>>>> @@ -190,15 +190,19 @@ void qemu_chr_send_event(CharDriverState *s, int
>>>> event)
>>>>         s->chr_send_event(s, event);
>>>>  }
>>>>
>>>> +static QemuChrHandlers null_handlers = {
>>>> +    /* All handlers are initialised to NULL */
>>>> +};
>>>> +
>>>>  void qemu_chr_add_handlers(CharDriverState *s,
>>>> -                           IOCanReadHandler *fd_can_read,
>>>> -                           IOReadHandler *fd_read,
>>>> -                           IOEventHandler *fd_event,
>>>> -                           void *opaque)
>>>> -{
>>>> -    s->chr_can_read = fd_can_read;
>>>> -    s->chr_read = fd_read;
>>>> -    s->chr_event = fd_event;
>>>> +                           QemuChrHandlers *handlers, void *opaque)
>>>> +{
>>>
>>> Here we could also check if (!s) and return if so. This would simplify
>>> the callers a bit.
>>
>> Simplified in what way?
>
> I assume for reducing the need to have to check s->chr != NULL everytime
> beforehand. It's safer and would save a lot on repetitive code as well.

Yes, that's what I meant.
Amit Shah - Jan. 13, 2011, 6:14 a.m.
On (Wed) Jan 12 2011 [19:03:58], Blue Swirl wrote:
> >>>> +static QemuChrHandlers null_handlers = {
> >>>> +    /* All handlers are initialised to NULL */
> >>>> +};
> >>>> +
> >>>>  void qemu_chr_add_handlers(CharDriverState *s,
> >>>> -                           IOCanReadHandler *fd_can_read,
> >>>> -                           IOReadHandler *fd_read,
> >>>> -                           IOEventHandler *fd_event,
> >>>> -                           void *opaque)
> >>>> -{
> >>>> -    s->chr_can_read = fd_can_read;
> >>>> -    s->chr_read = fd_read;
> >>>> -    s->chr_event = fd_event;
> >>>> +                           QemuChrHandlers *handlers, void *opaque)
> >>>> +{
> >>>
> >>> Here we could also check if (!s) and return if so. This would simplify
> >>> the callers a bit.
> >>
> >> Simplified in what way?
> >
> > I assume for reducing the need to have to check s->chr != NULL everytime
> > beforehand. It's safer and would save a lot on repetitive code as well.
> 
> Yes, that's what I meant.

OK, can be added, but I'm wondering if it makes sense (i.e., if an
assert would be better than return).

		Amit
Blue Swirl - Jan. 13, 2011, 9:29 p.m.
On Thu, Jan 13, 2011 at 6:14 AM, Amit Shah <amit.shah@redhat.com> wrote:
> On (Wed) Jan 12 2011 [19:03:58], Blue Swirl wrote:
>> >>>> +static QemuChrHandlers null_handlers = {
>> >>>> +    /* All handlers are initialised to NULL */
>> >>>> +};
>> >>>> +
>> >>>>  void qemu_chr_add_handlers(CharDriverState *s,
>> >>>> -                           IOCanReadHandler *fd_can_read,
>> >>>> -                           IOReadHandler *fd_read,
>> >>>> -                           IOEventHandler *fd_event,
>> >>>> -                           void *opaque)
>> >>>> -{
>> >>>> -    s->chr_can_read = fd_can_read;
>> >>>> -    s->chr_read = fd_read;
>> >>>> -    s->chr_event = fd_event;
>> >>>> +                           QemuChrHandlers *handlers, void *opaque)
>> >>>> +{
>> >>>
>> >>> Here we could also check if (!s) and return if so. This would simplify
>> >>> the callers a bit.
>> >>
>> >> Simplified in what way?
>> >
>> > I assume for reducing the need to have to check s->chr != NULL everytime
>> > beforehand. It's safer and would save a lot on repetitive code as well.
>>
>> Yes, that's what I meant.
>
> OK, can be added, but I'm wondering if it makes sense (i.e., if an
> assert would be better than return).

No, the idea is that the caller does not have to do the current NULL
checks anymore and can then possibly pass a NULL CharDriverState. This
is of course invalid, but to make things easier for the caller, such
calls are just to be ignored.

Patch

diff --git a/gdbstub.c b/gdbstub.c
index 0aa081b..b5ba2ef 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2627,6 +2627,12 @@  static void gdb_sigterm_handler(int signal)
 }
 #endif
 
+static QemuChrHandlers gdb_handlers = {
+    .fd_can_read = gdb_chr_can_receive,
+    .fd_read = gdb_chr_receive,
+    .fd_event = gdb_chr_event,
+};
+
 int gdbserver_start(const char *device)
 {
     GDBState *s;
@@ -2656,8 +2662,7 @@  int gdbserver_start(const char *device)
         if (!chr)
             return -1;
 
-        qemu_chr_add_handlers(chr, gdb_chr_can_receive, gdb_chr_receive,
-                              gdb_chr_event, NULL);
+        qemu_chr_add_handlers(chr, &gdb_handlers, NULL);
     }
 
     s = gdbserver_state;
diff --git a/hw/debugcon.c b/hw/debugcon.c
index 5ee6821..e79a595 100644
--- a/hw/debugcon.c
+++ b/hw/debugcon.c
@@ -73,7 +73,7 @@  static void debugcon_init_core(DebugconState *s)
         exit(1);
     }
 
-    qemu_chr_add_handlers(s->chr, NULL, NULL, NULL, s);
+    qemu_chr_add_handlers(s->chr, NULL, s);
 }
 
 static int debugcon_isa_initfn(ISADevice *dev)
diff --git a/hw/escc.c b/hw/escc.c
index ba60636..6be7b53 100644
--- a/hw/escc.c
+++ b/hw/escc.c
@@ -894,6 +894,12 @@  void slavio_serial_ms_kbd_init(target_phys_addr_t base, qemu_irq irq,
     sysbus_mmio_map(s, 0, base);
 }
 
+static QemuChrHandlers serial_handlers = {
+    .fd_can_read = serial_can_receive,
+    .fd_read = serial_receive1,
+    .fd_event = serial_event,
+};
+
 static int escc_init1(SysBusDevice *dev)
 {
     SerialState *s = FROM_SYSBUS(SerialState, dev);
@@ -907,8 +913,7 @@  static int escc_init1(SysBusDevice *dev)
         s->chn[i].chn = 1 - i;
         s->chn[i].clock = s->frequency / 2;
         if (s->chn[i].chr) {
-            qemu_chr_add_handlers(s->chn[i].chr, serial_can_receive,
-                                  serial_receive1, serial_event, &s->chn[i]);
+            qemu_chr_add_handlers(s->chn[i].chr, &serial_handlers, &s->chn[i]);
         }
     }
     s->chn[0].otherchn = &s->chn[1];
diff --git a/hw/etraxfs_ser.c b/hw/etraxfs_ser.c
index 2787ebd..4612c2b 100644
--- a/hw/etraxfs_ser.c
+++ b/hw/etraxfs_ser.c
@@ -190,6 +190,12 @@  static void serial_event(void *opaque, int event)
 
 }
 
+static QemuChrHandlers serial_handlers = {
+    .fd_can_read = serial_can_receive,
+    .fd_read = serial_receive,
+    .fd_event = serial_event,
+};
+
 static int etraxfs_ser_init(SysBusDevice *dev)
 {
     struct etrax_serial *s = FROM_SYSBUS(typeof (*s), dev);
@@ -204,10 +210,9 @@  static int etraxfs_ser_init(SysBusDevice *dev)
                                       DEVICE_NATIVE_ENDIAN);
     sysbus_init_mmio(dev, R_MAX * 4, ser_regs);
     s->chr = qdev_init_chardev(&dev->qdev);
-    if (s->chr)
-        qemu_chr_add_handlers(s->chr,
-                      serial_can_receive, serial_receive,
-                      serial_event, s);
+    if (s->chr) {
+        qemu_chr_add_handlers(s->chr, &serial_handlers, s);
+    }
     return 0;
 }
 
diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index 7b19a81..2a47e98 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -312,6 +312,18 @@  static void fake_irqfd(void *opaque, const uint8_t *buf, int size) {
     msix_notify(pdev, entry->vector);
 }
 
+static QemuChrHandlers ivshmem_msi_handlers = {
+    .fd_can_read = ivshmem_can_receive,
+    .fd_read = fake_irqfd,
+    .fd_event = ivshmem_event,
+};
+
+static QemuChrHandlers ivshmem_nomsi_handlers = {
+    .fd_can_read = ivshmem_can_receive,
+    .fd_read = ivshmem_receive,
+    .fd_event = ivshmem_event,
+};
+
 static CharDriverState* create_eventfd_chr_device(void * opaque, int eventfd,
                                                                     int vector)
 {
@@ -331,11 +343,10 @@  static CharDriverState* create_eventfd_chr_device(void * opaque, int eventfd,
         s->eventfd_table[vector].pdev = &s->dev;
         s->eventfd_table[vector].vector = vector;
 
-        qemu_chr_add_handlers(chr, ivshmem_can_receive, fake_irqfd,
-                      ivshmem_event, &s->eventfd_table[vector]);
+        qemu_chr_add_handlers(chr, &ivshmem_msi_handlers,
+                              &s->eventfd_table[vector]);
     } else {
-        qemu_chr_add_handlers(chr, ivshmem_can_receive, ivshmem_receive,
-                      ivshmem_event, s);
+        qemu_chr_add_handlers(chr, &ivshmem_nomsi_handlers, s);
     }
 
     return chr;
@@ -666,6 +677,12 @@  static int ivshmem_load(QEMUFile* f, void *opaque, int version_id)
     return 0;
 }
 
+static QemuChrHandlers ivshmem_handlers = {
+    .fd_can_read = ivshmem_can_receive,
+    .fd_read = ivshmem_read,
+    .fd_event = ivshmem_event,
+};
+
 static int pci_ivshmem_init(PCIDevice *dev)
 {
     IVShmemState *s = DO_UPCAST(IVShmemState, dev, dev);
@@ -754,8 +771,7 @@  static int pci_ivshmem_init(PCIDevice *dev)
 
         s->eventfd_chr = qemu_mallocz(s->vectors * sizeof(CharDriverState *));
 
-        qemu_chr_add_handlers(s->server_chr, ivshmem_can_receive, ivshmem_read,
-                     ivshmem_event, s);
+        qemu_chr_add_handlers(s->server_chr, &ivshmem_handlers, s);
     } else {
         /* just map the file immediately, we're not using a server */
         int fd;
diff --git a/hw/mcf_uart.c b/hw/mcf_uart.c
index db57096..f8ff526 100644
--- a/hw/mcf_uart.c
+++ b/hw/mcf_uart.c
@@ -268,6 +268,12 @@  static void mcf_uart_receive(void *opaque, const uint8_t *buf, int size)
     mcf_uart_push_byte(s, buf[0]);
 }
 
+static QemuChrHandlers mcf_uart_handlers = {
+    .fd_can_read = mcf_uart_can_receive,
+    .fd_read = mcf_uart_receive,
+    .fd_event = mcf_uart_event,
+};
+
 void *mcf_uart_init(qemu_irq irq, CharDriverState *chr)
 {
     mcf_uart_state *s;
@@ -276,8 +282,7 @@  void *mcf_uart_init(qemu_irq irq, CharDriverState *chr)
     s->chr = chr;
     s->irq = irq;
     if (chr) {
-        qemu_chr_add_handlers(chr, mcf_uart_can_receive, mcf_uart_receive,
-                              mcf_uart_event, s);
+        qemu_chr_add_handlers(chr, &mcf_uart_handlers, s);
     }
     mcf_uart_reset(s);
     return s;
diff --git a/hw/pl011.c b/hw/pl011.c
index 77f0dbf..107601a 100644
--- a/hw/pl011.c
+++ b/hw/pl011.c
@@ -286,6 +286,12 @@  static int pl011_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
+static QemuChrHandlers pl011_handlers = {
+    .fd_can_read = pl011_can_receive,
+    .fd_read = pl011_receive,
+    .fd_event = pl011_event,
+};
+
 static int pl011_init(SysBusDevice *dev, const unsigned char *id)
 {
     int iomemtype;
@@ -304,8 +310,7 @@  static int pl011_init(SysBusDevice *dev, const unsigned char *id)
     s->cr = 0x300;
     s->flags = 0x90;
     if (s->chr) {
-        qemu_chr_add_handlers(s->chr, pl011_can_receive, pl011_receive,
-                              pl011_event, s);
+        qemu_chr_add_handlers(s->chr, &pl011_handlers, s);
     }
     register_savevm(&dev->qdev, "pl011_uart", -1, 1, pl011_save, pl011_load, s);
     return 0;
diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c
index ab524a7..952732a 100644
--- a/hw/pxa2xx.c
+++ b/hw/pxa2xx.c
@@ -1995,6 +1995,12 @@  static int pxa2xx_fir_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
+static QemuChrHandlers pxa2xx_handlers = {
+    .fd_can_read = pxa2xx_fir_is_empty,
+    .fd_read = pxa2xx_fir_rx,
+    .fd_event = pxa2xx_fir_event,
+};
+
 static PXA2xxFIrState *pxa2xx_fir_init(target_phys_addr_t base,
                 qemu_irq irq, PXA2xxDMAState *dma,
                 CharDriverState *chr)
@@ -2013,10 +2019,9 @@  static PXA2xxFIrState *pxa2xx_fir_init(target_phys_addr_t base,
                     pxa2xx_fir_writefn, s, DEVICE_NATIVE_ENDIAN);
     cpu_register_physical_memory(base, 0x1000, iomemtype);
 
-    if (chr)
-        qemu_chr_add_handlers(chr, pxa2xx_fir_is_empty,
-                        pxa2xx_fir_rx, pxa2xx_fir_event, s);
-
+    if (chr) {
+        qemu_chr_add_handlers(chr, &pxa2xx_handlers, s);
+    }
     register_savevm(NULL, "pxa2xx_fir", 0, 0, pxa2xx_fir_save,
                     pxa2xx_fir_load, s);
 
diff --git a/hw/serial.c b/hw/serial.c
index 2c4af61..5ed4b9f 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -727,6 +727,12 @@  static void serial_reset(void *opaque)
     qemu_irq_lower(s->irq);
 }
 
+static QemuChrHandlers serial_handlers = {
+    .fd_can_read = serial_can_receive1,
+    .fd_read = serial_receive1,
+    .fd_event = serial_event,
+};
+
 static void serial_init_core(SerialState *s)
 {
     if (!s->chr) {
@@ -741,8 +747,7 @@  static void serial_init_core(SerialState *s)
 
     qemu_register_reset(serial_reset, s);
 
-    qemu_chr_add_handlers(s->chr, serial_can_receive1, serial_receive1,
-                          serial_event, s);
+    qemu_chr_add_handlers(s->chr, &serial_handlers, s);
 }
 
 /* Change the main reference oscillator frequency. */
diff --git a/hw/sh_serial.c b/hw/sh_serial.c
index 1bdc0a5..69ab6e3 100644
--- a/hw/sh_serial.c
+++ b/hw/sh_serial.c
@@ -363,6 +363,12 @@  static CPUWriteMemoryFunc * const sh_serial_writefn[] = {
     &sh_serial_write,
 };
 
+static QemuChrHandlers sh_serial_handlers = {
+    .fd_can_read = sh_serial_can_receive1,
+    .fd_read = sh_serial_receive1,
+    .fd_event = sh_serial_event,
+};
+
 void sh_serial_init (target_phys_addr_t base, int feat,
 		     uint32_t freq, CharDriverState *chr,
 		     qemu_irq eri_source,
@@ -402,9 +408,9 @@  void sh_serial_init (target_phys_addr_t base, int feat,
 
     s->chr = chr;
 
-    if (chr)
-        qemu_chr_add_handlers(chr, sh_serial_can_receive1, sh_serial_receive1,
-			      sh_serial_event, s);
+    if (chr) {
+        qemu_chr_add_handlers(chr, &sh_serial_handlers, s);
+    }
 
     s->eri = eri_source;
     s->rxi = rxi_source;
diff --git a/hw/syborg_serial.c b/hw/syborg_serial.c
index 34ce076..1e6b60b 100644
--- a/hw/syborg_serial.c
+++ b/hw/syborg_serial.c
@@ -315,6 +315,12 @@  static int syborg_serial_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
+static QemuChrHandlers syborg_serial_handlers = {
+    .fd_can_read = syborg_serial_can_receive,
+    .fd_read = syborg_serial_receive,
+    .fd_event = syborg_serial_event,
+};
+
 static int syborg_serial_init(SysBusDevice *dev)
 {
     SyborgSerialState *s = FROM_SYSBUS(SyborgSerialState, dev);
@@ -327,8 +333,7 @@  static int syborg_serial_init(SysBusDevice *dev)
     sysbus_init_mmio(dev, 0x1000, iomemtype);
     s->chr = qdev_init_chardev(&dev->qdev);
     if (s->chr) {
-        qemu_chr_add_handlers(s->chr, syborg_serial_can_receive,
-                              syborg_serial_receive, syborg_serial_event, s);
+        qemu_chr_add_handlers(s->chr, &syborg_serial_handlers, s);
     }
     if (s->fifo_size <= 0) {
         fprintf(stderr, "syborg_serial: fifo too small\n");
diff --git a/hw/usb-serial.c b/hw/usb-serial.c
index c19580f..697cb89 100644
--- a/hw/usb-serial.c
+++ b/hw/usb-serial.c
@@ -540,6 +540,12 @@  static void usb_serial_event(void *opaque, int event)
     }
 }
 
+static QemuChrHandlers usb_serial_handlers = {
+    .fd_can_read = usb_serial_can_read,
+    .fd_read = usb_serial_read,
+    .fd_event = usb_serial_event,
+};
+
 static int usb_serial_initfn(USBDevice *dev)
 {
     USBSerialState *s = DO_UPCAST(USBSerialState, dev, dev);
@@ -550,8 +556,7 @@  static int usb_serial_initfn(USBDevice *dev)
         return -1;
     }
 
-    qemu_chr_add_handlers(s->cs, usb_serial_can_read, usb_serial_read,
-                          usb_serial_event, s);
+    qemu_chr_add_handlers(s->cs, &usb_serial_handlers, s);
     usb_serial_handle_reset(dev);
     return 0;
 }
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index d0b9354..3bd2b79 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -57,13 +57,18 @@  static void chr_event(void *opaque, int event)
     }
 }
 
+static QemuChrHandlers chr_handlers = {
+    .fd_can_read = chr_can_read,
+    .fd_read = chr_read,
+    .fd_event = chr_event,
+};
+
 static int generic_port_init(VirtConsole *vcon, VirtIOSerialDevice *dev)
 {
     vcon->port.info = dev->info;
 
     if (vcon->chr) {
-        qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event,
-                              vcon);
+        qemu_chr_add_handlers(vcon->chr, &chr_handlers, vcon);
         vcon->port.info->have_data = flush_buf;
     }
     return 0;
diff --git a/hw/xen_console.c b/hw/xen_console.c
index d2261f4..f490a5a 100644
--- a/hw/xen_console.c
+++ b/hw/xen_console.c
@@ -202,6 +202,11 @@  static int con_init(struct XenDevice *xendev)
     return 0;
 }
 
+static QemuChrHandlers xencons_handlers = {
+    .fd_can_read = xencons_can_receive,
+    .fd_read = xencons_receive,
+};
+
 static int con_connect(struct XenDevice *xendev)
 {
     struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
@@ -222,9 +227,9 @@  static int con_connect(struct XenDevice *xendev)
 	return -1;
 
     xen_be_bind_evtchn(&con->xendev);
-    if (con->chr)
-        qemu_chr_add_handlers(con->chr, xencons_can_receive, xencons_receive,
-                              NULL, con);
+    if (con->chr) {
+        qemu_chr_add_handlers(con->chr, &xencons_handlers, con);
+    }
 
     xen_be_printf(xendev, 1, "ring mfn %d, remote port %d, local port %d, limit %zd\n",
 		  con->ring_ref,
@@ -238,8 +243,9 @@  static void con_disconnect(struct XenDevice *xendev)
 {
     struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
 
-    if (con->chr)
-        qemu_chr_add_handlers(con->chr, NULL, NULL, NULL, NULL);
+    if (con->chr) {
+        qemu_chr_add_handlers(con->chr, NULL, NULL);
+    }
     xen_be_unbind_evtchn(&con->xendev);
 
     if (con->sring) {
diff --git a/hw/xilinx_uartlite.c b/hw/xilinx_uartlite.c
index 9b94e98..9ae84f7 100644
--- a/hw/xilinx_uartlite.c
+++ b/hw/xilinx_uartlite.c
@@ -193,6 +193,12 @@  static void uart_event(void *opaque, int event)
 
 }
 
+static QemuChrHandlers uart_handlers = {
+    .fd_can_read = uart_can_rx,
+    .fd_read = uart_rx,
+    .fd_event = uart_event,
+};
+
 static int xilinx_uartlite_init(SysBusDevice *dev)
 {
     struct xlx_uartlite *s = FROM_SYSBUS(typeof (*s), dev);
@@ -206,8 +212,9 @@  static int xilinx_uartlite_init(SysBusDevice *dev)
     sysbus_init_mmio(dev, R_MAX * 4, uart_regs);
 
     s->chr = qdev_init_chardev(&dev->qdev);
-    if (s->chr)
-        qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
+    if (s->chr) {
+        qemu_chr_add_handlers(s->chr, &uart_handlers, s);
+    }
     return 0;
 }
 
diff --git a/monitor.c b/monitor.c
index f258000..0c16970 100644
--- a/monitor.c
+++ b/monitor.c
@@ -5110,6 +5110,18 @@  static void monitor_event(void *opaque, int event)
  * End:
  */
 
+static QemuChrHandlers monitor_handlers = {
+    .fd_can_read = monitor_can_read,
+    .fd_read = monitor_read,
+    .fd_event = monitor_event,
+};
+
+static QemuChrHandlers monitor_control_handlers = {
+    .fd_can_read = monitor_can_read,
+    .fd_read = monitor_control_read,
+    .fd_event = monitor_control_event,
+};
+
 void monitor_init(CharDriverState *chr, int flags)
 {
     static int is_first_init = 1;
@@ -5131,12 +5143,11 @@  void monitor_init(CharDriverState *chr, int flags)
 
     if (monitor_ctrl_mode(mon)) {
         mon->mc = qemu_mallocz(sizeof(MonitorControl));
+
         /* Control mode requires special handlers */
-        qemu_chr_add_handlers(chr, monitor_can_read, monitor_control_read,
-                              monitor_control_event, mon);
+        qemu_chr_add_handlers(chr, &monitor_control_handlers, mon);
     } else {
-        qemu_chr_add_handlers(chr, monitor_can_read, monitor_read,
-                              monitor_event, mon);
+        qemu_chr_add_handlers(chr, &monitor_handlers, mon);
     }
 
     QLIST_INSERT_HEAD(&mon_list, mon, entry);
diff --git a/net/slirp.c b/net/slirp.c
index b41c60a..52e9a5c 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -577,6 +577,11 @@  static void guestfwd_read(void *opaque, const uint8_t *buf, int size)
     slirp_socket_recv(fwd->slirp, fwd->server, fwd->port, buf, size);
 }
 
+static QemuChrHandlers guestfwd_handlers = {
+    .fd_can_read = guestfwd_can_read,
+    .fd_read = guestfwd_read,
+};
+
 static int slirp_guestfwd(SlirpState *s, const char *config_str,
                           int legacy_format)
 {
@@ -633,8 +638,7 @@  static int slirp_guestfwd(SlirpState *s, const char *config_str,
     fwd->port = port;
     fwd->slirp = s->slirp;
 
-    qemu_chr_add_handlers(fwd->hd, guestfwd_can_read, guestfwd_read,
-                          NULL, fwd);
+    qemu_chr_add_handlers(fwd->hd, &guestfwd_handlers, fwd);
     return 0;
 
  fail_syntax:
diff --git a/qemu-char.c b/qemu-char.c
index edc9ad6..d9a1df3 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -190,15 +190,19 @@  void qemu_chr_send_event(CharDriverState *s, int event)
         s->chr_send_event(s, event);
 }
 
+static QemuChrHandlers null_handlers = {
+    /* All handlers are initialised to NULL */
+};
+
 void qemu_chr_add_handlers(CharDriverState *s,
-                           IOCanReadHandler *fd_can_read,
-                           IOReadHandler *fd_read,
-                           IOEventHandler *fd_event,
-                           void *opaque)
-{
-    s->chr_can_read = fd_can_read;
-    s->chr_read = fd_read;
-    s->chr_event = fd_event;
+                           QemuChrHandlers *handlers, void *opaque)
+{
+    if (!handlers) {
+        handlers = &null_handlers;
+    }
+    s->chr_can_read = handlers->fd_can_read;
+    s->chr_read = handlers->fd_read;
+    s->chr_event = handlers->fd_event;
     s->handler_opaque = opaque;
     if (s->chr_update_read_handler)
         s->chr_update_read_handler(s);
@@ -436,6 +440,12 @@  static void mux_chr_event(void *opaque, int event)
         mux_chr_send_event(d, i, event);
 }
 
+static QemuChrHandlers mux_chr_handlers = {
+    .fd_can_read = mux_chr_can_read,
+    .fd_read = mux_chr_read,
+    .fd_event = mux_chr_event,
+};
+
 static void mux_chr_update_read_handler(CharDriverState *chr)
 {
     MuxDriver *d = chr->opaque;
@@ -450,8 +460,7 @@  static void mux_chr_update_read_handler(CharDriverState *chr)
     d->chr_event[d->mux_cnt] = chr->chr_event;
     /* Fix up the real driver with mux routines */
     if (d->mux_cnt == 0) {
-        qemu_chr_add_handlers(d->drv, mux_chr_can_read, mux_chr_read,
-                              mux_chr_event, chr);
+        qemu_chr_add_handlers(d->drv, &mux_chr_handlers, chr);
     }
     if (d->focus != -1) {
         mux_chr_send_event(d, d->focus, CHR_EVENT_MUX_OUT);
diff --git a/qemu-char.h b/qemu-char.h
index e6ee6c4..8ed0ffd 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -72,6 +72,13 @@  struct CharDriverState {
     QTAILQ_ENTRY(CharDriverState) next;
 };
 
+typedef struct QemuChrHandlers {
+    IOCanReadHandler *fd_can_read;
+    IOReadHandler *fd_read;
+    IOHandler *fd_write_unblocked;
+    IOEventHandler *fd_event;
+} QemuChrHandlers;
+
 QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
 CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
                                     void (*init)(struct CharDriverState *s));
@@ -81,10 +88,7 @@  void qemu_chr_printf(CharDriverState *s, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
 int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len);
 void qemu_chr_send_event(CharDriverState *s, int event);
-void qemu_chr_add_handlers(CharDriverState *s,
-                           IOCanReadHandler *fd_can_read,
-                           IOReadHandler *fd_read,
-                           IOEventHandler *fd_event,
+void qemu_chr_add_handlers(CharDriverState *s, QemuChrHandlers *handlers,
                            void *opaque);
 int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg);
 void qemu_chr_generic_open(CharDriverState *s);