All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/1] char: Add a QemuChrHandlers struct to initialise chardev handlers
@ 2012-02-10 13:19 Amit Shah
  2012-02-10 13:28 ` Anthony Liguori
  0 siblings, 1 reply; 6+ messages in thread
From: Amit Shah @ 2012-02-10 13:19 UTC (permalink / raw)
  To: qemu list; +Cc: Amit Shah

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/ccid-card-passthru.c |   11 +++++++----
 hw/debugcon.c           |    2 +-
 hw/escc.c               |    9 +++++++--
 hw/etraxfs_ser.c        |   13 +++++++++----
 hw/grlib_apbuart.c      |   12 +++++++-----
 hw/ivshmem.c            |   28 ++++++++++++++++++++++------
 hw/lm32_juart.c         |    8 +++++++-
 hw/lm32_uart.c          |    8 +++++++-
 hw/mcf_uart.c           |    9 +++++++--
 hw/milkymist-uart.c     |    8 +++++++-
 hw/pl011.c              |    9 +++++++--
 hw/pxa2xx.c             |   13 +++++++++----
 hw/qdev-properties.c    |    2 +-
 hw/serial.c             |    9 +++++++--
 hw/sh_serial.c          |   12 +++++++++---
 hw/spapr_vty.c          |    9 ++++++---
 hw/strongarm.c          |   12 +++++++-----
 hw/usb-serial.c         |    9 +++++++--
 hw/virtio-console.c     |    9 +++++++--
 hw/xen_console.c        |   16 +++++++++++-----
 hw/xilinx_uartlite.c    |   11 +++++++++--
 monitor.c               |   18 ++++++++++++++----
 net/slirp.c             |    8 ++++++--
 qemu-char.c             |   32 ++++++++++++++++++++++----------
 qemu-char.h             |   12 ++++++++----
 usb-redir.c             |    9 +++++++--
 27 files changed, 225 insertions(+), 82 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 7d470b6..612147e 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2862,6 +2862,12 @@ static void gdb_sigterm_handler(int signal)
 }
 #endif
 
+static const 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;
@@ -2891,8 +2897,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/ccid-card-passthru.c b/hw/ccid-card-passthru.c
index a7006ca..652e207 100644
--- a/hw/ccid-card-passthru.c
+++ b/hw/ccid-card-passthru.c
@@ -274,6 +274,12 @@ static const uint8_t *passthru_get_atr(CCIDCardState *base, uint32_t *len)
     return card->atr;
 }
 
+static const QemuChrHandlers passthru_handlers = {
+    .fd_can_read = ccid_card_vscard_can_read,
+    .fd_read = ccid_card_vscard_read,
+    .fd_event = ccid_card_vscard_event,
+};
+
 static int passthru_initfn(CCIDCardState *base)
 {
     PassthruState *card = DO_UPCAST(PassthruState, base, base);
@@ -282,10 +288,7 @@ static int passthru_initfn(CCIDCardState *base)
     card->vscard_in_hdr = 0;
     if (card->cs) {
         DPRINTF(card, D_INFO, "initing chardev\n");
-        qemu_chr_add_handlers(card->cs,
-            ccid_card_vscard_can_read,
-            ccid_card_vscard_read,
-            ccid_card_vscard_event, card);
+        qemu_chr_add_handlers(card->cs, &passthru_handlers, card);
         ccid_card_vscard_send_init(card);
     } else {
         error_report("missing chardev");
diff --git a/hw/debugcon.c b/hw/debugcon.c
index 3903b26..a1ee7e1 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 9fe282f..3e6c5a0 100644
--- a/hw/escc.c
+++ b/hw/escc.c
@@ -867,6 +867,12 @@ void slavio_serial_ms_kbd_init(target_phys_addr_t base, qemu_irq irq,
     sysbus_mmio_map(s, 0, base);
 }
 
+static const 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);
@@ -879,8 +885,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 567cb8c..1021003 100644
--- a/hw/etraxfs_ser.c
+++ b/hw/etraxfs_ser.c
@@ -208,6 +208,12 @@ static void etraxfs_ser_reset(DeviceState *d)
 
 }
 
+static const 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);
@@ -217,10 +223,9 @@ static int etraxfs_ser_init(SysBusDevice *dev)
     sysbus_init_mmio(dev, &s->mmio);
 
     s->chr = qemu_char_get_next_serial();
-    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/grlib_apbuart.c b/hw/grlib_apbuart.c
index 89de2d8..f25e264 100644
--- a/hw/grlib_apbuart.c
+++ b/hw/grlib_apbuart.c
@@ -222,15 +222,17 @@ static const MemoryRegionOps grlib_apbuart_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static const QemuChrHandlers grlib_handlers = {
+    .fd_can_read = grlib_apbuart_can_receive,
+    .fd_read = grlib_apbuart_receive,
+    .fd_event = grlib_apbuart_event,
+};
+
 static int grlib_apbuart_init(SysBusDevice *dev)
 {
     UART *uart = FROM_SYSBUS(typeof(*uart), dev);
 
-    qemu_chr_add_handlers(uart->chr,
-                          grlib_apbuart_can_receive,
-                          grlib_apbuart_receive,
-                          grlib_apbuart_event,
-                          uart);
+    qemu_chr_add_handlers(uart->chr, &grlib_handlers, uart);
 
     sysbus_init_irq(dev, &uart->irq);
 
diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index 6f017d4..16c0a85 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -279,6 +279,18 @@ static void fake_irqfd(void *opaque, const uint8_t *buf, int size) {
     msix_notify(pdev, entry->vector);
 }
 
+static const QemuChrHandlers ivshmem_handlers = {
+    .fd_can_read = ivshmem_can_receive,
+    .fd_read = ivshmem_receive,
+    .fd_event = ivshmem_event,
+};
+
+static const QemuChrHandlers ivshmem_msi_handlers = {
+    .fd_can_read = ivshmem_can_receive,
+    .fd_read = fake_irqfd,
+    .fd_event = ivshmem_event,
+};
+
 static CharDriverState* create_eventfd_chr_device(void * opaque, int eventfd,
                                                                     int vector)
 {
@@ -298,11 +310,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_handlers, s);
     }
 
     return chr;
@@ -619,6 +630,12 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int version_id)
     return 0;
 }
 
+static const QemuChrHandlers ivshmem_server_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);
@@ -708,8 +725,7 @@ static int pci_ivshmem_init(PCIDevice *dev)
 
         s->eventfd_chr = g_malloc0(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_server_handlers, s);
     } else {
         /* just map the file immediately, we're not using a server */
         int fd;
diff --git a/hw/lm32_juart.c b/hw/lm32_juart.c
index 38dd282..ccefc52 100644
--- a/hw/lm32_juart.c
+++ b/hw/lm32_juart.c
@@ -110,13 +110,19 @@ static void juart_reset(DeviceState *d)
     s->jrx = 0;
 }
 
+static const QemuChrHandlers juart_handlers = {
+    .fd_can_read = juart_can_rx,
+    .fd_read = juart_rx,
+    .fd_event = juart_event,
+};
+
 static int lm32_juart_init(SysBusDevice *dev)
 {
     LM32JuartState *s = FROM_SYSBUS(typeof(*s), dev);
 
     s->chr = qemu_char_get_next_serial();
     if (s->chr) {
-        qemu_chr_add_handlers(s->chr, juart_can_rx, juart_rx, juart_event, s);
+        qemu_chr_add_handlers(s->chr, &juart_handlers, s);
     }
 
     return 0;
diff --git a/hw/lm32_uart.c b/hw/lm32_uart.c
index 630ccb7..df7a0de 100644
--- a/hw/lm32_uart.c
+++ b/hw/lm32_uart.c
@@ -243,6 +243,12 @@ static void uart_reset(DeviceState *d)
     s->regs[R_LSR] = LSR_THRE | LSR_TEMT;
 }
 
+static const QemuChrHandlers uart_handlers = {
+    .fd_can_read = uart_can_rx,
+    .fd_read = uart_rx,
+    .fd_event = uart_event,
+};
+
 static int lm32_uart_init(SysBusDevice *dev)
 {
     LM32UartState *s = FROM_SYSBUS(typeof(*s), dev);
@@ -254,7 +260,7 @@ static int lm32_uart_init(SysBusDevice *dev)
 
     s->chr = qemu_char_get_next_serial();
     if (s->chr) {
-        qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
+        qemu_chr_add_handlers(s->chr, &uart_handlers, s);
     }
 
     return 0;
diff --git a/hw/mcf_uart.c b/hw/mcf_uart.c
index ec6a87f..f52fb96 100644
--- a/hw/mcf_uart.c
+++ b/hw/mcf_uart.c
@@ -272,6 +272,12 @@ static void mcf_uart_receive(void *opaque, const uint8_t *buf, int size)
     mcf_uart_push_byte(s, buf[0]);
 }
 
+static const 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;
@@ -280,8 +286,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/milkymist-uart.c b/hw/milkymist-uart.c
index f9a229c..d7d97ae 100644
--- a/hw/milkymist-uart.c
+++ b/hw/milkymist-uart.c
@@ -189,6 +189,12 @@ static void milkymist_uart_reset(DeviceState *d)
     s->regs[R_STAT] = STAT_THRE;
 }
 
+static const QemuChrHandlers uart_handlers = {
+    .fd_can_read = uart_can_rx,
+    .fd_read = uart_rx,
+    .fd_event = uart_event,
+};
+
 static int milkymist_uart_init(SysBusDevice *dev)
 {
     MilkymistUartState *s = FROM_SYSBUS(typeof(*s), dev);
@@ -201,7 +207,7 @@ static int milkymist_uart_init(SysBusDevice *dev)
 
     s->chr = qemu_char_get_next_serial();
     if (s->chr) {
-        qemu_chr_add_handlers(s->chr, uart_can_rx, uart_rx, uart_event, s);
+        qemu_chr_add_handlers(s->chr, &uart_handlers, s);
     }
 
     return 0;
diff --git a/hw/pl011.c b/hw/pl011.c
index 752cbf9..22971e6 100644
--- a/hw/pl011.c
+++ b/hw/pl011.c
@@ -256,6 +256,12 @@ static const VMStateDescription vmstate_pl011 = {
     }
 };
 
+static const 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)
 {
     pl011_state *s = FROM_SYSBUS(pl011_state, dev);
@@ -271,8 +277,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);
     }
     vmstate_register(&dev->qdev, -1, &vmstate_pl011, s);
     return 0;
diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c
index 244c614..0921653 100644
--- a/hw/pxa2xx.c
+++ b/hw/pxa2xx.c
@@ -2012,6 +2012,12 @@ static int pxa2xx_fir_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
+static const 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(MemoryRegion *sysmem,
                 target_phys_addr_t base,
                 qemu_irq irq, qemu_irq rx_dma, qemu_irq tx_dma,
@@ -2030,10 +2036,9 @@ static PXA2xxFIrState *pxa2xx_fir_init(MemoryRegion *sysmem,
     memory_region_init_io(&s->iomem, &pxa2xx_fir_ops, s, "pxa2xx-fir", 0x1000);
     memory_region_add_subregion(sysmem, base, &s->iomem);
 
-    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/qdev-properties.c b/hw/qdev-properties.c
index b6d6fcf..82106b6 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -663,7 +663,7 @@ static void release_chr(Object *obj, const char *name, void *opaque)
     CharDriverState **ptr = qdev_get_prop_ptr(dev, prop);
 
     if (*ptr) {
-        qemu_chr_add_handlers(*ptr, NULL, NULL, NULL, NULL);
+        qemu_chr_add_handlers(*ptr, NULL, NULL);
     }
 }
 
diff --git a/hw/serial.c b/hw/serial.c
index 82917e2..6b6ddd9 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -728,6 +728,12 @@ static void serial_reset(void *opaque)
     qemu_irq_lower(s->irq);
 }
 
+static const 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) {
@@ -742,8 +748,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 43b0eb1..c322784 100644
--- a/hw/sh_serial.c
+++ b/hw/sh_serial.c
@@ -350,6 +350,12 @@ static const MemoryRegionOps sh_serial_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static const 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(MemoryRegion *sysmem,
                     target_phys_addr_t base, int feat,
                     uint32_t freq, CharDriverState *chr,
@@ -394,9 +400,9 @@ void sh_serial_init(MemoryRegion *sysmem,
 
     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/spapr_vty.c b/hw/spapr_vty.c
index a954e7d..418fd6f 100644
--- a/hw/spapr_vty.c
+++ b/hw/spapr_vty.c
@@ -54,6 +54,11 @@ void vty_putchars(VIOsPAPRDevice *sdev, uint8_t *buf, int len)
     qemu_chr_fe_write(dev->chardev, buf, len);
 }
 
+static const QemuChrHandlers vty_handlers = {
+    .fd_can_read = vty_can_receive,
+    .fd_read = vty_receive,
+};
+
 static int spapr_vty_init(VIOsPAPRDevice *sdev)
 {
     VIOsPAPRVTYDevice *dev = (VIOsPAPRVTYDevice *)sdev;
@@ -62,9 +67,7 @@ static int spapr_vty_init(VIOsPAPRDevice *sdev)
         fprintf(stderr, "spapr-vty: Can't create vty without a chardev!\n");
         exit(1);
     }
-
-    qemu_chr_add_handlers(dev->chardev, vty_can_receive,
-                          vty_receive, NULL, dev);
+    qemu_chr_add_handlers(dev->chardev, &vty_handlers, dev);
 
     return 0;
 }
diff --git a/hw/strongarm.c b/hw/strongarm.c
index 8d2e7eb..12265ea 100644
--- a/hw/strongarm.c
+++ b/hw/strongarm.c
@@ -1199,6 +1199,12 @@ static const MemoryRegionOps strongarm_uart_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+static const QemuChrHandlers strongarm_uart_handlers = {
+    .fd_can_read = strongarm_uart_can_receive,
+    .fd_read = strongarm_uart_receive,
+    .fd_event = strongarm_uart_event,
+};
+
 static int strongarm_uart_init(SysBusDevice *dev)
 {
     StrongARMUARTState *s = FROM_SYSBUS(StrongARMUARTState, dev);
@@ -1211,11 +1217,7 @@ static int strongarm_uart_init(SysBusDevice *dev)
     s->tx_timer = qemu_new_timer_ns(vm_clock, strongarm_uart_tx, s);
 
     if (s->chr) {
-        qemu_chr_add_handlers(s->chr,
-                        strongarm_uart_can_receive,
-                        strongarm_uart_receive,
-                        strongarm_uart_event,
-                        s);
+        qemu_chr_add_handlers(s->chr, &strongarm_uart_handlers, s);
     }
 
     return 0;
diff --git a/hw/usb-serial.c b/hw/usb-serial.c
index c2cb6d2..bdf4511 100644
--- a/hw/usb-serial.c
+++ b/hw/usb-serial.c
@@ -475,6 +475,12 @@ static void usb_serial_event(void *opaque, int event)
     }
 }
 
+static const 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);
@@ -486,8 +492,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 4f2c3e4..eaa77b7 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -106,6 +106,12 @@ static void chr_event(void *opaque, int event)
     }
 }
 
+static const QemuChrHandlers chr_handlers = {
+    .fd_can_read = chr_can_read,
+    .fd_read = chr_read,
+    .fd_event = chr_event,
+};
+
 static int virtconsole_initfn(VirtIOSerialPort *port)
 {
     VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
@@ -117,8 +123,7 @@ static int virtconsole_initfn(VirtIOSerialPort *port)
     }
 
     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);
     }
 
     return 0;
diff --git a/hw/xen_console.c b/hw/xen_console.c
index edcb31c..2ba74f0 100644
--- a/hw/xen_console.c
+++ b/hw/xen_console.c
@@ -212,6 +212,11 @@ out:
     return ret;
 }
 
+static const QemuChrHandlers xencons_handlers = {
+    .fd_can_read = xencons_can_receive,
+    .fd_read = xencons_receive,
+};
+
 static int con_initialise(struct XenDevice *xendev)
 {
     struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
@@ -232,9 +237,9 @@ static int con_initialise(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,
@@ -248,8 +253,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 1c2b908..2bb6d64 100644
--- a/hw/xilinx_uartlite.c
+++ b/hw/xilinx_uartlite.c
@@ -195,6 +195,12 @@ static void uart_event(void *opaque, int event)
 
 }
 
+static const 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, &s->mmio);
 
     s->chr = qemu_char_get_next_serial();
-    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 aadbdcb..8541e55 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4551,6 +4551,18 @@ static void sortcmdlist(void)
  * End:
  */
 
+static const QemuChrHandlers monitor_handlers = {
+    .fd_can_read = monitor_can_read,
+    .fd_read = monitor_read,
+    .fd_event = monitor_event,
+};
+
+static const 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;
@@ -4573,12 +4585,10 @@ void monitor_init(CharDriverState *chr, int flags)
     if (monitor_ctrl_mode(mon)) {
         mon->mc = g_malloc0(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);
         qemu_chr_fe_set_echo(chr, true);
     } 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 18e07ba..6348a05 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -576,6 +576,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 const 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)
 {
@@ -632,8 +637,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 b1d80dd..ea05222 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -189,19 +189,26 @@ void qemu_chr_fe_printf(CharDriverState *s, const char *fmt, ...)
     va_end(ap);
 }
 
+static const 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)
+                           const QemuChrHandlers *handlers, void *opaque)
 {
-    if (!opaque && !fd_can_read && !fd_read && !fd_event) {
+    if (!s) {
+        return;
+    }
+    if (!opaque && !handlers) {
         /* chr driver being released. */
         ++s->avail_connections;
     }
-    s->chr_can_read = fd_can_read;
-    s->chr_read = fd_read;
-    s->chr_event = fd_event;
+    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);
@@ -441,6 +448,12 @@ static void mux_chr_event(void *opaque, int event)
         mux_chr_send_event(d, i, event);
 }
 
+static const 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;
@@ -455,8 +468,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 486644b..acc5c19 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -77,6 +77,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;
+
 /**
  * @qemu_chr_new_from_opts:
  *
@@ -222,10 +229,7 @@ void qemu_chr_be_write(CharDriverState *s, uint8_t *buf, int len);
  */
 void qemu_chr_be_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, const QemuChrHandlers *handlers,
                            void *opaque);
 
 void qemu_chr_generic_open(CharDriverState *s);
diff --git a/usb-redir.c b/usb-redir.c
index 303292a..2b6e388 100644
--- a/usb-redir.c
+++ b/usb-redir.c
@@ -866,6 +866,12 @@ static void usbredir_chardev_event(void *opaque, int event)
     }
 }
 
+static const QemuChrHandlers chr_handlers = {
+    .fd_can_read = usbredir_chardev_can_read,
+    .fd_read = usbredir_chardev_read,
+    .fd_event = usbredir_chardev_event,
+};
+
 /*
  * init + destroy
  */
@@ -893,8 +899,7 @@ static int usbredir_initfn(USBDevice *udev)
 
     /* Let the backend know we are ready */
     qemu_chr_fe_open(dev->cs);
-    qemu_chr_add_handlers(dev->cs, usbredir_chardev_can_read,
-                          usbredir_chardev_read, usbredir_chardev_event, dev);
+    qemu_chr_add_handlers(dev->cs, &chr_handlers, dev);
 
     return 0;
 }
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1] char: Add a QemuChrHandlers struct to initialise chardev handlers
  2012-02-10 13:19 [Qemu-devel] [PATCH 1/1] char: Add a QemuChrHandlers struct to initialise chardev handlers Amit Shah
@ 2012-02-10 13:28 ` Anthony Liguori
  2012-02-10 17:09   ` Amit Shah
  0 siblings, 1 reply; 6+ messages in thread
From: Anthony Liguori @ 2012-02-10 13:28 UTC (permalink / raw)
  To: Amit Shah; +Cc: qemu list

On 02/10/2012 07:19 AM, 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.
>
> Signed-off-by: Amit Shah<amit.shah@redhat.com>

Why?

> ---
>   gdbstub.c               |    9 +++++++--
>   hw/ccid-card-passthru.c |   11 +++++++----
>   hw/debugcon.c           |    2 +-
>   hw/escc.c               |    9 +++++++--
>   hw/etraxfs_ser.c        |   13 +++++++++----
>   hw/grlib_apbuart.c      |   12 +++++++-----
>   hw/ivshmem.c            |   28 ++++++++++++++++++++++------
>   hw/lm32_juart.c         |    8 +++++++-
>   hw/lm32_uart.c          |    8 +++++++-
>   hw/mcf_uart.c           |    9 +++++++--
>   hw/milkymist-uart.c     |    8 +++++++-
>   hw/pl011.c              |    9 +++++++--
>   hw/pxa2xx.c             |   13 +++++++++----
>   hw/qdev-properties.c    |    2 +-
>   hw/serial.c             |    9 +++++++--
>   hw/sh_serial.c          |   12 +++++++++---
>   hw/spapr_vty.c          |    9 ++++++---
>   hw/strongarm.c          |   12 +++++++-----
>   hw/usb-serial.c         |    9 +++++++--
>   hw/virtio-console.c     |    9 +++++++--
>   hw/xen_console.c        |   16 +++++++++++-----
>   hw/xilinx_uartlite.c    |   11 +++++++++--
>   monitor.c               |   18 ++++++++++++++----
>   net/slirp.c             |    8 ++++++--
>   qemu-char.c             |   32 ++++++++++++++++++++++----------
>   qemu-char.h             |   12 ++++++++----
>   usb-redir.c             |    9 +++++++--
>   27 files changed, 225 insertions(+), 82 deletions(-)

It's not a win in terms of code size.  If you plan on introducing additional 
handlers, perhaps you should include this in that series where it's more 
appropriately justified.

As a change on it's own, it doesn't seem to make a lot of sense.

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1] char: Add a QemuChrHandlers struct to initialise chardev handlers
  2012-02-10 13:28 ` Anthony Liguori
@ 2012-02-10 17:09   ` Amit Shah
  2012-02-10 17:22     ` Anthony Liguori
  0 siblings, 1 reply; 6+ messages in thread
From: Amit Shah @ 2012-02-10 17:09 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu list

On (Fri) 10 Feb 2012 [07:28:19], Anthony Liguori wrote:
> On 02/10/2012 07:19 AM, 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.
> >
> >Signed-off-by: Amit Shah<amit.shah@redhat.com>
> 
> Why?

It's a good cleanup.

> It's not a win in terms of code size.  If you plan on introducing
> additional handlers, perhaps you should include this in that series
> where it's more appropriately justified.
> 
> As a change on it's own, it doesn't seem to make a lot of sense.

Makes the code much easier to look at.  Can't really compare on code
size, since there's zero change in the resulting binary, but the code
just becomes more readable and manageable.

		Amit

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1] char: Add a QemuChrHandlers struct to initialise chardev handlers
  2012-02-10 17:09   ` Amit Shah
@ 2012-02-10 17:22     ` Anthony Liguori
  2012-02-10 18:08       ` Kevin Wolf
  0 siblings, 1 reply; 6+ messages in thread
From: Anthony Liguori @ 2012-02-10 17:22 UTC (permalink / raw)
  To: Amit Shah; +Cc: qemu list

On 02/10/2012 11:09 AM, Amit Shah wrote:
> On (Fri) 10 Feb 2012 [07:28:19], Anthony Liguori wrote:
>> On 02/10/2012 07:19 AM, 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.
>>>
>>> Signed-off-by: Amit Shah<amit.shah@redhat.com>
>>
>> Why?
>
> It's a good cleanup.
>
>> It's not a win in terms of code size.  If you plan on introducing
>> additional handlers, perhaps you should include this in that series
>> where it's more appropriately justified.
>>
>> As a change on it's own, it doesn't seem to make a lot of sense.
>
> Makes the code much easier to look at.  Can't really compare on code
> size, since there's zero change in the resulting binary, but the code
> just becomes more readable and manageable.

It's not more readable IMHO.  You've taken function call arguments from the 
place they naturally belong (in the function call) and placed them somewhere else.

More importantly, this isn't a pattern we use in QEMU anywhere.

Regards,

Anthony Liguori

>
> 		Amit

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1] char: Add a QemuChrHandlers struct to initialise chardev handlers
  2012-02-10 17:22     ` Anthony Liguori
@ 2012-02-10 18:08       ` Kevin Wolf
  2012-02-10 18:18         ` Anthony Liguori
  0 siblings, 1 reply; 6+ messages in thread
From: Kevin Wolf @ 2012-02-10 18:08 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Amit Shah, qemu list

Am 10.02.2012 18:22, schrieb Anthony Liguori:
> On 02/10/2012 11:09 AM, Amit Shah wrote:
>> On (Fri) 10 Feb 2012 [07:28:19], Anthony Liguori wrote:
>>> On 02/10/2012 07:19 AM, 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.
>>>>
>>>> Signed-off-by: Amit Shah<amit.shah@redhat.com>
>>>
>>> Why?
>>
>> It's a good cleanup.
>>
>>> It's not a win in terms of code size.  If you plan on introducing
>>> additional handlers, perhaps you should include this in that series
>>> where it's more appropriately justified.
>>>
>>> As a change on it's own, it doesn't seem to make a lot of sense.
>>
>> Makes the code much easier to look at.  Can't really compare on code
>> size, since there's zero change in the resulting binary, but the code
>> just becomes more readable and manageable.
> 
> It's not more readable IMHO.  You've taken function call arguments from the 
> place they naturally belong (in the function call) and placed them somewhere else.
> 
> More importantly, this isn't a pattern we use in QEMU anywhere.

The obvious next step is to replace
CharDriverState.chr_can_read/read/event with a CharDriverState.ops that
refers to a CharDriverHandler struct, and suddenly you have a pattern
that is used a lot in QEMU (and that indeed increases readability in my
opinion).

Kevin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1] char: Add a QemuChrHandlers struct to initialise chardev handlers
  2012-02-10 18:08       ` Kevin Wolf
@ 2012-02-10 18:18         ` Anthony Liguori
  0 siblings, 0 replies; 6+ messages in thread
From: Anthony Liguori @ 2012-02-10 18:18 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: Amit Shah, qemu list

On 02/10/2012 12:08 PM, Kevin Wolf wrote:
> Am 10.02.2012 18:22, schrieb Anthony Liguori:
>> On 02/10/2012 11:09 AM, Amit Shah wrote:
>>> On (Fri) 10 Feb 2012 [07:28:19], Anthony Liguori wrote:
>>>> On 02/10/2012 07:19 AM, 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.
>>>>>
>>>>> Signed-off-by: Amit Shah<amit.shah@redhat.com>
>>>>
>>>> Why?
>>>
>>> It's a good cleanup.
>>>
>>>> It's not a win in terms of code size.  If you plan on introducing
>>>> additional handlers, perhaps you should include this in that series
>>>> where it's more appropriately justified.
>>>>
>>>> As a change on it's own, it doesn't seem to make a lot of sense.
>>>
>>> Makes the code much easier to look at.  Can't really compare on code
>>> size, since there's zero change in the resulting binary, but the code
>>> just becomes more readable and manageable.
>>
>> It's not more readable IMHO.  You've taken function call arguments from the
>> place they naturally belong (in the function call) and placed them somewhere else.
>>
>> More importantly, this isn't a pattern we use in QEMU anywhere.
>
> The obvious next step is to replace
> CharDriverState.chr_can_read/read/event with a CharDriverState.ops that
> refers to a CharDriverHandler struct, and suddenly you have a pattern
> that is used a lot in QEMU (and that indeed increases readability in my
> opinion).

I actually think that you need to change chr_can_read/read/event to take 
CharDriverHandler as the first argument instead of CharDriverState, then drop 
the handle_opaque and refactor callers appropriately.

But then at this stage, we should drop NULL handlers as a disconnect mechanism 
and switch to a qemu_chr_fe_connect(), qemu_chr_fe_disconnect().  And that 
indicates that we should call it CharFrontEnd instead of CharDriverHandler.

So in such a series, I don't think this patch would even survive.

Regards,

Anthony Liguori

>
> Kevin
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-02-10 18:18 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-10 13:19 [Qemu-devel] [PATCH 1/1] char: Add a QemuChrHandlers struct to initialise chardev handlers Amit Shah
2012-02-10 13:28 ` Anthony Liguori
2012-02-10 17:09   ` Amit Shah
2012-02-10 17:22     ` Anthony Liguori
2012-02-10 18:08       ` Kevin Wolf
2012-02-10 18:18         ` Anthony Liguori

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.