[Qemu-devel] [PATCH 01/13] slirp: convert save/load function to visitor interface

[Michael Roth]

Where possible common routines are used for both input and output, thus
save on lines of code, theoretically. The added lines here are mostly
due to extra logic for each save/load routine to manipulate strings into
a unique field name for each saved field, and in some cases a few extra
Visitor calls to due list/struct i/o. With some reworking we can
probably optimize all of these to reduce the amount of added code.

Signed-off-by: Michael Roth <address@hidden>
---
 slirp/slirp.c |  366 +++++++++++++++++++++++++++++++++------------------------
 1 files changed, 213 insertions(+), 153 deletions(-)

diff --git a/slirp/slirp.c b/slirp/slirp.c
index 19d69eb..9631890 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -26,6 +26,9 @@
 #include "qemu-char.h"
 #include "slirp.h"
 #include "hw/hw.h"
+#include "qemu-error.h"
+
+#define SLIRP_DELIMITER 42 /* used to separate slirp instances in save/load */
 
 /* host loopback address */
 struct in_addr loopback_addr;
@@ -871,98 +874,147 @@ void slirp_socket_recv(Slirp *slirp, struct in_addr 
guest_addr, int guest_port,
         tcp_output(sototcpcb(so));
 }
 
-static void slirp_tcp_save(QEMUFile *f, struct tcpcb *tp)
+static void slirp_tcp_visit(Visitor *v, struct tcpcb *tp, Error *err)
 {
     int i;
+    int16_t *ptr;
+
+    visit_type_int16(v, &tp->t_state, "t_state", &err);
+    ptr = tp->t_timer;
+    visit_start_array(v, (void **)&ptr, "t_timer", TCPT_NTIMERS, sizeof(*ptr), 
&err);
+    for (i = 0; i < TCPT_NTIMERS; ++i) {
+        visit_type_int16(v, &ptr[i], NULL, &err);
+    }
+    visit_end_array(v, &err);
+    visit_type_int16(v, &tp->t_rxtshift, "t_rxtshift", &err);
+    visit_type_int16(v, &tp->t_rxtcur, "t_rxtcur", &err);
+    visit_type_int16(v, &tp->t_dupacks, "t_dupacks", &err);
+    visit_type_uint16(v, &tp->t_maxseg, "t_maxseg", &err);
+    visit_type_uint8(v, (uint8_t *)&tp->t_force, "t_force", &err);
+    visit_type_uint16(v, &tp->t_flags, "t_flags", &err);
+    visit_type_uint32(v, &tp->snd_una, "snd_una", &err);
+    visit_type_uint32(v, &tp->snd_nxt, "snd_nxt", &err);
+    visit_type_uint32(v, &tp->snd_up, "snd_up", &err);
+    visit_type_uint32(v, &tp->snd_wl1, "snd_wl1", &err);
+    visit_type_uint32(v, &tp->snd_wl2, "snd_wl2", &err);
+    visit_type_uint32(v, &tp->iss, "iss", &err);
+    visit_type_uint32(v, &tp->snd_wnd, "snd_wnd", &err);
+    visit_type_uint32(v, &tp->rcv_wnd, "rcv_wnd", &err);
+    visit_type_uint32(v, &tp->rcv_nxt, "rcv_nxt", &err);
+    visit_type_uint32(v, &tp->rcv_up, "rcv_up", &err);
+    visit_type_uint32(v, &tp->irs, "irs", &err);
+    visit_type_uint32(v, &tp->rcv_adv, "rcv_adv", &err);
+    visit_type_uint32(v, &tp->snd_max, "snd_max", &err);
+    visit_type_uint32(v, &tp->snd_cwnd, "snd_cwnd", &err);
+    visit_type_uint32(v, &tp->snd_ssthresh, "snd_ssthresh", &err);
+    visit_type_int16(v, &tp->t_idle, "t_idle", &err);
+    visit_type_int16(v, &tp->t_rtt, "t_rtt", &err);
+    visit_type_uint32(v, &tp->t_rtseq, "t_rtseq", &err);
+    visit_type_int16(v, &tp->t_srtt, "t_srtt", &err);
+    visit_type_int16(v, &tp->t_rttvar, "t_rttvar", &err);
+    visit_type_uint16(v, &tp->t_rttmin, "t_rttmin", &err);
+    visit_type_uint32(v, &tp->max_sndwnd, "max_sndwnd", &err);
+    visit_type_uint8(v, (uint8_t *)&tp->t_oobflags, "t_oobflags", &err);
+    visit_type_uint8(v, (uint8_t *)&tp->t_iobc, "t_iobc", &err);
+    visit_type_int16(v, &tp->t_softerror, "t_softerror", &err);
+    visit_type_uint8(v, &tp->snd_scale, "snd_scale", &err);
+    visit_type_uint8(v, &tp->rcv_scale, "rcv_scale", &err);
+    visit_type_uint8(v, &tp->request_r_scale, "request_r_scale", &err);
+    visit_type_uint8(v, &tp->requested_s_scale, "requested_s_scale", &err);
+    visit_type_uint32(v, &tp->ts_recent, "ts_recent", &err);
+    visit_type_uint32(v, &tp->ts_recent_age, "ts_recent_age", &err);
+    visit_type_uint32(v, &tp->last_ack_sent, "last_ack_sent", &err);
+}
 
-    qemu_put_sbe16(f, tp->t_state);
-    for (i = 0; i < TCPT_NTIMERS; i++)
-        qemu_put_sbe16(f, tp->t_timer[i]);
-    qemu_put_sbe16(f, tp->t_rxtshift);
-    qemu_put_sbe16(f, tp->t_rxtcur);
-    qemu_put_sbe16(f, tp->t_dupacks);
-    qemu_put_be16(f, tp->t_maxseg);
-    qemu_put_sbyte(f, tp->t_force);
-    qemu_put_be16(f, tp->t_flags);
-    qemu_put_be32(f, tp->snd_una);
-    qemu_put_be32(f, tp->snd_nxt);
-    qemu_put_be32(f, tp->snd_up);
-    qemu_put_be32(f, tp->snd_wl1);
-    qemu_put_be32(f, tp->snd_wl2);
-    qemu_put_be32(f, tp->iss);
-    qemu_put_be32(f, tp->snd_wnd);
-    qemu_put_be32(f, tp->rcv_wnd);
-    qemu_put_be32(f, tp->rcv_nxt);
-    qemu_put_be32(f, tp->rcv_up);
-    qemu_put_be32(f, tp->irs);
-    qemu_put_be32(f, tp->rcv_adv);
-    qemu_put_be32(f, tp->snd_max);
-    qemu_put_be32(f, tp->snd_cwnd);
-    qemu_put_be32(f, tp->snd_ssthresh);
-    qemu_put_sbe16(f, tp->t_idle);
-    qemu_put_sbe16(f, tp->t_rtt);
-    qemu_put_be32(f, tp->t_rtseq);
-    qemu_put_sbe16(f, tp->t_srtt);
-    qemu_put_sbe16(f, tp->t_rttvar);
-    qemu_put_be16(f, tp->t_rttmin);
-    qemu_put_be32(f, tp->max_sndwnd);
-    qemu_put_byte(f, tp->t_oobflags);
-    qemu_put_byte(f, tp->t_iobc);
-    qemu_put_sbe16(f, tp->t_softerror);
-    qemu_put_byte(f, tp->snd_scale);
-    qemu_put_byte(f, tp->rcv_scale);
-    qemu_put_byte(f, tp->request_r_scale);
-    qemu_put_byte(f, tp->requested_s_scale);
-    qemu_put_be32(f, tp->ts_recent);
-    qemu_put_be32(f, tp->ts_recent_age);
-    qemu_put_be32(f, tp->last_ack_sent);
+static void slirp_tcp_save(Visitor *v, struct tcpcb *tp, Error *err)
+{
+    slirp_tcp_visit(v, tp, err);
 }
 
-static void slirp_sbuf_save(QEMUFile *f, struct sbuf *sbuf)
+static void slirp_sbuf_save(Visitor *v, struct sbuf *sbuf, Error *err)
 {
-    uint32_t off;
+    int32_t off;
+    int i;
 
-    qemu_put_be32(f, sbuf->sb_cc);
-    qemu_put_be32(f, sbuf->sb_datalen);
+    visit_type_uint32(v, &sbuf->sb_cc, "sb_cc", &err);
+    visit_type_uint32(v, &sbuf->sb_datalen, "sb_datalen", &err);
     off = (uint32_t)(sbuf->sb_wptr - sbuf->sb_data);
-    qemu_put_sbe32(f, off);
+    visit_type_int32(v, &off, "sb_wptr_off", &err);
     off = (uint32_t)(sbuf->sb_rptr - sbuf->sb_data);
-    qemu_put_sbe32(f, off);
-    qemu_put_buffer(f, (unsigned char*)sbuf->sb_data, sbuf->sb_datalen);
+    visit_type_int32(v, &off, "sb_rptr_off", &err);
+
+    visit_start_array(v, (void **)&sbuf->sb_data, "sb_data",
+                      sbuf->sb_datalen, sizeof(*sbuf->sb_data), &err);
+    for (i = 0; i < sbuf->sb_datalen; i++) {
+        visit_type_uint8(v, (uint8_t *)&sbuf->sb_data[i], NULL, &err);
+    }
+    visit_end_array(v, &err);
 }
 
-static void slirp_socket_save(QEMUFile *f, struct socket *so)
+static void slirp_socket_visit(Visitor *v, struct socket *so, Error *err)
 {
-    qemu_put_be32(f, so->so_urgc);
-    qemu_put_be32(f, so->so_faddr.s_addr);
-    qemu_put_be32(f, so->so_laddr.s_addr);
-    qemu_put_be16(f, so->so_fport);
-    qemu_put_be16(f, so->so_lport);
-    qemu_put_byte(f, so->so_iptos);
-    qemu_put_byte(f, so->so_emu);
-    qemu_put_byte(f, so->so_type);
-    qemu_put_be32(f, so->so_state);
-    slirp_sbuf_save(f, &so->so_rcv);
-    slirp_sbuf_save(f, &so->so_snd);
-    slirp_tcp_save(f, so->so_tcpcb);
+    visit_type_int32(v, &so->so_urgc, "so_urgc", &err);
+    /* TODO: use a nested struct for these guys */
+    visit_type_uint32(v, &so->so_faddr.s_addr, "so_faddr.s_addr", &err);
+    visit_type_uint32(v, &so->so_laddr.s_addr, "so_laddr.so_urgc", &err);
+    visit_type_uint16(v, &so->so_fport, "so_fport", &err);
+    visit_type_uint16(v, &so->so_lport, "so_lport", &err);
+    visit_type_uint8(v, &so->so_iptos, "so_iptos", &err);
+    visit_type_uint8(v, &so->so_emu, "so_emu", &err);
+    visit_type_uint8(v, &so->so_type, "so_type", &err);
+    visit_type_int32(v, &so->so_state, "so_state", &err);
 }
 
-static void slirp_bootp_save(QEMUFile *f, Slirp *slirp)
+static void slirp_socket_save(Visitor *v, struct socket *so, Error *err)
 {
-    int i;
+    slirp_socket_visit(v, so, err);
+
+    visit_start_struct(v, NULL, NULL, "so_rcv", 0, &err);
+    slirp_sbuf_save(v, &so->so_rcv, err);
+    visit_end_struct(v, &err);
+
+    visit_start_struct(v, NULL, NULL, "so_snd", 0, &err);
+    slirp_sbuf_save(v, &so->so_snd, err);
+    visit_end_struct(v, &err);
+
+    visit_start_struct(v, NULL, NULL, "so_tcpcb", 0, &err);
+    slirp_tcp_save(v, so->so_tcpcb, err);
+    visit_end_struct(v, &err);
+}
 
+static void slirp_bootp_visit(Visitor *v, Slirp *slirp, Error *err)
+{
+    int i, j;
+    BOOTPClient *bootp_clients = slirp->bootp_clients;
+    uint8_t *macaddr;
+    visit_start_array(v, (void **)&bootp_clients, "bootp_clients",
+                      NB_BOOTP_CLIENTS, sizeof(BOOTPClient), &err);
     for (i = 0; i < NB_BOOTP_CLIENTS; i++) {
-        qemu_put_be16(f, slirp->bootp_clients[i].allocated);
-        qemu_put_buffer(f, slirp->bootp_clients[i].macaddr, 6);
+        visit_start_struct(v, NULL, NULL, NULL, 0, &err);
+        visit_type_uint16(v, &bootp_clients[i].allocated, "allocated", &err);
+        macaddr = slirp->bootp_clients[i].macaddr;
+        visit_start_array(v, (void **)&macaddr, "macaddr", 6, 1, &err);
+        for (j = 0; j < 6; j++) {
+            visit_type_uint8(v, &macaddr[j], NULL, &err);
+        }
+        visit_end_array(v, &err);
+        visit_end_struct(v, &err);
     }
+    visit_end_array(v, &err);
 }
 
 static void slirp_state_save(QEMUFile *f, void *opaque)
 {
     Slirp *slirp = opaque;
     struct ex_list *ex_ptr;
+    uint8_t padding = SLIRP_DELIMITER;
+    Visitor *v = qemu_file_get_visitor(f);
+    Error *err = NULL;
 
-    for (ex_ptr = slirp->exec_list; ex_ptr; ex_ptr = ex_ptr->ex_next)
+    visit_start_struct(v, NULL, NULL, "slirp", 0, &err);
+
+    visit_start_list(v, "sockets", &err);
+    for (ex_ptr = slirp->exec_list; ex_ptr; ex_ptr = ex_ptr->ex_next) {
         if (ex_ptr->ex_pty == 3) {
             struct socket *so;
             so = slirp_find_ctl_socket(slirp, ex_ptr->ex_addr,
@@ -970,70 +1022,47 @@ static void slirp_state_save(QEMUFile *f, void *opaque)
             if (!so)
                 continue;
 
-            qemu_put_byte(f, 42);
-            slirp_socket_save(f, so);
+            visit_start_struct(v, NULL, NULL, NULL, 0, &err);
+            visit_type_uint8(v, &padding, "padding", &err);
+            slirp_socket_save(v, so, err);
+            visit_end_struct(v, &err);
         }
-    qemu_put_byte(f, 0);
+    }
+    padding = 0;
+    visit_start_struct(v, NULL, NULL, NULL, 0, &err);
+    visit_type_uint8(v, &padding, "padding", &err);
+    visit_end_struct(v, &err);
+    visit_end_list(v, &err); /* slirp sockets */
+
 
-    qemu_put_be16(f, slirp->ip_id);
+    visit_type_uint16(v, &slirp->ip_id, "ip_id", &err);
 
-    slirp_bootp_save(f, slirp);
+    visit_start_struct(v, NULL, NULL, "bootp", 0, &err);
+    slirp_bootp_visit(v, slirp, err);
+    visit_end_struct(v, &err);
+
+    if (err) {
+        error_report("error saving slirp state: %s", error_get_pretty(err));
+        error_free(err);
+    }
+
+    visit_end_struct(v, &err); /* slirp state object */
 }
 
-static void slirp_tcp_load(QEMUFile *f, struct tcpcb *tp)
+static void slirp_tcp_load(Visitor *v, struct tcpcb *tp, Error *err)
 {
-    int i;
+    slirp_tcp_visit(v, tp, err);
 
-    tp->t_state = qemu_get_sbe16(f);
-    for (i = 0; i < TCPT_NTIMERS; i++)
-        tp->t_timer[i] = qemu_get_sbe16(f);
-    tp->t_rxtshift = qemu_get_sbe16(f);
-    tp->t_rxtcur = qemu_get_sbe16(f);
-    tp->t_dupacks = qemu_get_sbe16(f);
-    tp->t_maxseg = qemu_get_be16(f);
-    tp->t_force = qemu_get_sbyte(f);
-    tp->t_flags = qemu_get_be16(f);
-    tp->snd_una = qemu_get_be32(f);
-    tp->snd_nxt = qemu_get_be32(f);
-    tp->snd_up = qemu_get_be32(f);
-    tp->snd_wl1 = qemu_get_be32(f);
-    tp->snd_wl2 = qemu_get_be32(f);
-    tp->iss = qemu_get_be32(f);
-    tp->snd_wnd = qemu_get_be32(f);
-    tp->rcv_wnd = qemu_get_be32(f);
-    tp->rcv_nxt = qemu_get_be32(f);
-    tp->rcv_up = qemu_get_be32(f);
-    tp->irs = qemu_get_be32(f);
-    tp->rcv_adv = qemu_get_be32(f);
-    tp->snd_max = qemu_get_be32(f);
-    tp->snd_cwnd = qemu_get_be32(f);
-    tp->snd_ssthresh = qemu_get_be32(f);
-    tp->t_idle = qemu_get_sbe16(f);
-    tp->t_rtt = qemu_get_sbe16(f);
-    tp->t_rtseq = qemu_get_be32(f);
-    tp->t_srtt = qemu_get_sbe16(f);
-    tp->t_rttvar = qemu_get_sbe16(f);
-    tp->t_rttmin = qemu_get_be16(f);
-    tp->max_sndwnd = qemu_get_be32(f);
-    tp->t_oobflags = qemu_get_byte(f);
-    tp->t_iobc = qemu_get_byte(f);
-    tp->t_softerror = qemu_get_sbe16(f);
-    tp->snd_scale = qemu_get_byte(f);
-    tp->rcv_scale = qemu_get_byte(f);
-    tp->request_r_scale = qemu_get_byte(f);
-    tp->requested_s_scale = qemu_get_byte(f);
-    tp->ts_recent = qemu_get_be32(f);
-    tp->ts_recent_age = qemu_get_be32(f);
-    tp->last_ack_sent = qemu_get_be32(f);
     tcp_template(tp);
 }
 
-static int slirp_sbuf_load(QEMUFile *f, struct sbuf *sbuf)
+static int slirp_sbuf_load(Visitor *v, struct sbuf *sbuf, Error *err)
 {
     uint32_t off, sb_cc, sb_datalen;
+    int i;
 
-    sb_cc = qemu_get_be32(f);
-    sb_datalen = qemu_get_be32(f);
+    visit_type_uint32(v, &sb_cc, "sb_cc", &err);
+    visit_type_uint32(v, &sb_datalen, "sb_datalen", &err);
 
     sbreserve(sbuf, sb_datalen);
 
@@ -1042,64 +1071,82 @@ static int slirp_sbuf_load(QEMUFile *f, struct sbuf 
*sbuf)
 
     sbuf->sb_cc = sb_cc;
 
-    off = qemu_get_sbe32(f);
+    visit_type_uint32(v, &off, "sb_wptr_off", &err);
     sbuf->sb_wptr = sbuf->sb_data + off;
-    off = qemu_get_sbe32(f);
+    visit_type_uint32(v, &off, "sb_rptr_off", &err);
     sbuf->sb_rptr = sbuf->sb_data + off;
-    qemu_get_buffer(f, (unsigned char*)sbuf->sb_data, sbuf->sb_datalen);
+
+    visit_start_array(v, (void **)&sbuf->sb_data, "sb_data",
+                      sbuf->sb_datalen, sizeof(*sbuf->sb_data), &err);
+    for (i = 0; i < sbuf->sb_datalen; i++) {
+        visit_type_uint8(v, (uint8_t *)&sbuf->sb_data[i], NULL, &err);
+    }
+    visit_end_array(v, &err);
 
     return 0;
 }
 
-static int slirp_socket_load(QEMUFile *f, struct socket *so)
+static int slirp_socket_load(Visitor *v, struct socket *so, Error *err)
 {
+    assert(v);
     if (tcp_attach(so) < 0)
         return -ENOMEM;
 
-    so->so_urgc = qemu_get_be32(f);
-    so->so_faddr.s_addr = qemu_get_be32(f);
-    so->so_laddr.s_addr = qemu_get_be32(f);
-    so->so_fport = qemu_get_be16(f);
-    so->so_lport = qemu_get_be16(f);
-    so->so_iptos = qemu_get_byte(f);
-    so->so_emu = qemu_get_byte(f);
-    so->so_type = qemu_get_byte(f);
-    so->so_state = qemu_get_be32(f);
-    if (slirp_sbuf_load(f, &so->so_rcv) < 0)
-        return -ENOMEM;
-    if (slirp_sbuf_load(f, &so->so_snd) < 0)
+    slirp_socket_visit(v, so, err);
+
+    visit_start_struct(v, NULL, NULL, "so_rcv", 0, &err);
+    if (slirp_sbuf_load(v, &so->so_rcv, err) < 0) {
         return -ENOMEM;
-    slirp_tcp_load(f, so->so_tcpcb);
+    }
+    visit_end_struct(v, &err);
 
-    return 0;
-}
+    visit_start_struct(v, NULL, NULL, "so_snd", 0, &err);
+    if (slirp_sbuf_load(v, &so->so_snd, err) < 0) {
+        return -ENOMEM;
+    }
+    visit_end_struct(v, &err);
 
-static void slirp_bootp_load(QEMUFile *f, Slirp *slirp)
-{
-    int i;
+    visit_start_struct(v, NULL, NULL, "so_tcpcb", 0, &err);
+    slirp_tcp_load(v, so->so_tcpcb, err);
+    visit_end_struct(v, &err);
 
-    for (i = 0; i < NB_BOOTP_CLIENTS; i++) {
-        slirp->bootp_clients[i].allocated = qemu_get_be16(f);
-        qemu_get_buffer(f, slirp->bootp_clients[i].macaddr, 6);
-    }
+    return 0;
 }
 
 static int slirp_state_load(QEMUFile *f, void *opaque, int version_id)
 {
     Slirp *slirp = opaque;
     struct ex_list *ex_ptr;
+    Visitor *v = qemu_file_get_visitor(f);
+    Error *err = NULL;
+    uint8_t padding;
+
+    visit_start_struct(v, NULL, NULL, "slirp", 0, &err);
 
-    while (qemu_get_byte(f)) {
+    visit_start_list(v, "sockets", &err);
+    while (1) {
         int ret;
-        struct socket *so = socreate(slirp);
+        struct socket *so;
+
+        visit_start_struct(v, NULL, NULL, NULL, 0, &err);
 
-        if (!so)
+        visit_type_uint8(v, &padding, "padding", &err);
+        if (padding != SLIRP_DELIMITER) {
+            visit_end_struct(v, &err);
+            break;
+        }
+
+        so = socreate(slirp);
+        if (!so) {
             return -ENOMEM;
+        }
 
-        ret = slirp_socket_load(f, so);
+        ret = slirp_socket_load(v, so, err);
+        visit_end_struct(v, &err);
 
-        if (ret < 0)
+        if (ret < 0) {
             return ret;
+        }
 
         if ((so->so_faddr.s_addr & slirp->vnetwork_mask.s_addr) !=
             slirp->vnetwork_addr.s_addr) {
@@ -1112,19 +1159,32 @@ static int slirp_state_load(QEMUFile *f, void *opaque, 
int version_id)
                 break;
             }
         }
-        if (!ex_ptr)
+        if (!ex_ptr) {
             return -EINVAL;
+        }
 
         so->extra = (void *)ex_ptr->ex_exec;
-    }
+        visit_type_uint8(v, &padding, "padding", &err);
+    };
+    visit_end_list(v, &err); /* slirp sockets */
 
     if (version_id >= 2) {
-        slirp->ip_id = qemu_get_be16(f);
+        visit_type_uint16(v, &slirp->ip_id, "ip_id", &err);
     }
 
     if (version_id >= 3) {
-        slirp_bootp_load(f, slirp);
+        visit_start_struct(v, NULL, NULL, "bootp", 0, &err);
+        slirp_bootp_visit(v, slirp, err);
+        visit_end_struct(v, &err);
     }
 
+    if (err) {
+        error_report("error loading slirp state: %s", error_get_pretty(err));
+        error_free(err);
+        return -EINVAL;
+    }
+
+    visit_end_struct(v, &err); /* slirp state object */
+
     return 0;
 }
-- 
1.7.4.1