Patchwork [5/5] iohandlers: Add IOHandlerOps struct

login
register
mail settings
Submitter Amit Shah
Date Jan. 13, 2011, 1 p.m.
Message ID <54967ebf79fdbfdb79f12c2d93608ac260a47795.1294923288.git.amit.shah@redhat.com>
Download mbox | patch
Permalink /patch/78751/
State New
Headers show

Comments

Amit Shah - Jan. 13, 2011, 1 p.m.
Collect all the handlers in a IOHandlerOps struct instead of being
passed one at a time to each function.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 qemu-char.h |    7 ++-----
 vl.c        |   51 ++++++++++++++++++++++++++++++++-------------------
 2 files changed, 34 insertions(+), 24 deletions(-)
Gerd Hoffmann - Jan. 13, 2011, 2:02 p.m.
Hi,

> +    ioh->ops.fd_read_poll = ops->fd_read_poll;
> +    ioh->ops.fd_read = ops->fd_read;
> +    ioh->ops.fd_write = ops->fd_write;

You can write this as "ioh->ops = *ops" btw.

I guess the long-term plan (to be committed after killing the last user 
of the old interface) is to store a pointer to the ops struct instead of 
copying it?

cheers,
   Gerd
Amit Shah - Jan. 13, 2011, 2:09 p.m.
On (Thu) Jan 13 2011 [15:02:37], Gerd Hoffmann wrote:
>   Hi,
> 
> >+    ioh->ops.fd_read_poll = ops->fd_read_poll;
> >+    ioh->ops.fd_read = ops->fd_read;
> >+    ioh->ops.fd_write = ops->fd_write;
> 
> You can write this as "ioh->ops = *ops" btw.
> 
> I guess the long-term plan (to be committed after killing the last
> user of the old interface) is to store a pointer to the ops struct
> instead of copying it?

Yes, that's what I have in mind.

		Amit

Patch

diff --git a/qemu-char.h b/qemu-char.h
index e88a108..ebf9cd1 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -108,12 +108,9 @@  QString *qemu_chr_mem_to_qs(CharDriverState *chr);
 size_t qemu_chr_mem_osize(const CharDriverState *chr);
 
 /* async I/O support */
+typedef struct IOHandlerOps IOHandlerOps;
 
-int assign_fd_handlers(int fd,
-                       IOCanReadHandler *fd_read_poll,
-                       IOHandler *fd_read,
-                       IOHandler *fd_write,
-                       void *opaque);
+int assign_fd_handlers(int fd, const IOHandlerOps *ops, void *opaque);
 void remove_fd_handlers(int fd);
 int set_read_poll_fd_action(int fd, bool enable);
 int set_read_fd_action(int fd, bool enable);
diff --git a/vl.c b/vl.c
index 42ec36a..d2ef63f 100644
--- a/vl.c
+++ b/vl.c
@@ -1007,11 +1007,15 @@  void pcmcia_info(Monitor *mon)
 /***********************************************************/
 /* I/O handling */
 
-typedef struct IOHandlerRecord {
-    int fd;
+struct IOHandlerOps {
     IOCanReadHandler *fd_read_poll;
     IOHandler *fd_read;
     IOHandler *fd_write;
+};
+
+typedef struct IOHandlerRecord {
+    int fd;
+    IOHandlerOps ops;
     int deleted;
     void *opaque;
     bool read_poll_enabled;
@@ -1025,6 +1029,10 @@  typedef struct IOHandlerRecord {
 static QLIST_HEAD(, IOHandlerRecord) io_handlers =
     QLIST_HEAD_INITIALIZER(io_handlers);
 
+static const IOHandlerOps null_ioh_ops = {
+    /* All ops are NULL */
+};
+
 static IOHandlerRecord *get_iohandler(int fd)
 {
     IOHandlerRecord *ioh;
@@ -1039,17 +1047,16 @@  static IOHandlerRecord *get_iohandler(int fd)
 
 /* XXX: fd_read_poll should be suppressed, but an API change is
    necessary in the character devices to suppress fd_can_read(). */
-int assign_fd_handlers(int fd,
-                       IOCanReadHandler *fd_read_poll,
-                       IOHandler *fd_read,
-                       IOHandler *fd_write,
-                       void *opaque)
+int assign_fd_handlers(int fd, const IOHandlerOps *ops, void *opaque)
 {
     IOHandlerRecord *ioh;
 
     ioh = get_iohandler(fd);
 
-    if (ioh && !fd_read && !fd_write) {
+    if (!ops) {
+        ops = &null_ioh_ops;
+    }
+    if (ioh && !ops->fd_read && !ops->fd_write) {
         ioh->deleted = 1;
         return 0;
     }
@@ -1060,9 +1067,9 @@  int assign_fd_handlers(int fd,
 
         ioh->fd = fd;
     }
-    ioh->fd_read_poll = fd_read_poll;
-    ioh->fd_read = fd_read;
-    ioh->fd_write = fd_write;
+    ioh->ops.fd_read_poll = ops->fd_read_poll;
+    ioh->ops.fd_read = ops->fd_read;
+    ioh->ops.fd_write = ops->fd_write;
     ioh->opaque = opaque;
     ioh->deleted = 0;
     ioh->read_poll_enabled = ioh->read_enabled = ioh->write_enabled = false;
@@ -1072,7 +1079,7 @@  int assign_fd_handlers(int fd,
 
 void remove_fd_handlers(int fd)
 {
-    assign_fd_handlers(fd, NULL, NULL, NULL, NULL);
+    assign_fd_handlers(fd, NULL, NULL);
 }
 
 int set_read_poll_fd_action(int fd, bool enable)
@@ -1086,7 +1093,7 @@  int set_read_poll_fd_action(int fd, bool enable)
     }
 
     ioh->read_poll_enabled = false;
-    if (enable && ioh->fd_read_poll) {
+    if (enable && ioh->ops.fd_read_poll) {
         ioh->read_poll_enabled = true;
     }
 
@@ -1104,7 +1111,7 @@  int set_read_fd_action(int fd, bool enable)
     }
 
     ioh->read_enabled = false;
-    if (enable && ioh->fd_read) {
+    if (enable && ioh->ops.fd_read) {
         ioh->read_enabled = true;
     }
 
@@ -1122,7 +1129,7 @@  int set_write_fd_action(int fd, bool enable)
     }
 
     ioh->write_enabled = false;
-    if (enable && ioh->fd_write) {
+    if (enable && ioh->ops.fd_write) {
         ioh->write_enabled = true;
     }
 
@@ -1135,7 +1142,13 @@  int qemu_set_fd_handler2(int fd,
                          IOHandler *fd_write,
                          void *opaque)
 {
-    assign_fd_handlers(fd, fd_read_poll, fd_read, fd_write, opaque);
+    IOHandlerOps ops;
+
+    ops.fd_read_poll = fd_read_poll;
+    ops.fd_read = fd_read;
+    ops.fd_write = fd_write;
+    assign_fd_handlers(fd, &ops, opaque);
+
     set_read_poll_fd_action(fd, true);
     set_read_fd_action(fd, true);
     set_write_fd_action(fd, true);
@@ -1404,7 +1417,7 @@  void main_loop_wait(int nonblocking)
         if (ioh->deleted)
             continue;
         if (ioh->read_enabled &&
-            (!ioh->read_poll_enabled || ioh->fd_read_poll(ioh->opaque) != 0)) {
+            (!ioh->read_poll_enabled || ioh->ops.fd_read_poll(ioh->opaque) != 0)) {
             FD_SET(ioh->fd, &rfds);
             if (ioh->fd > nfds)
                 nfds = ioh->fd;
@@ -1429,10 +1442,10 @@  void main_loop_wait(int nonblocking)
 
         QLIST_FOREACH_SAFE(ioh, &io_handlers, next, pioh) {
             if (!ioh->deleted && ioh->read_enabled && FD_ISSET(ioh->fd, &rfds)) {
-                ioh->fd_read(ioh->opaque);
+                ioh->ops.fd_read(ioh->opaque);
             }
             if (!ioh->deleted && ioh->write_enabled && FD_ISSET(ioh->fd, &wfds)) {
-                ioh->fd_write(ioh->opaque);
+                ioh->ops.fd_write(ioh->opaque);
             }
 
             /* Do this last in case read/write handlers marked it for deletion */