[RFC,8/8] slirp: convert save/load function to visitor interface

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 <mdroth@linux.vnet.ibm.com>
---
 slirp/slirp.c |  366 +++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 216 insertions(+), 150 deletions(-)

On 09/19/2011 09:41 AM, Michael Roth wrote:
> 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<mdroth@linux.vnet.ibm.com>
> ---
>   slirp/slirp.c |  366 +++++++++++++++++++++++++++++++++-----------------------
>   1 files changed, 216 insertions(+), 150 deletions(-)
>
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index 19d69eb..8783626 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,96 +874,171 @@ 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, const char *pfield, Error *err)
>   {
> -    int i;
> -
> -    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);
> +    char f[128];
> +    int i, l = 0;
> +    int16_t *ptr;
> +
> +    if (pfield) {
> +        assert(strlen(pfield)<  sizeof(f));
> +        strcpy(f, pfield);
> +        l = strlen(pfield);
> +    }
> +
> +    visit_type_int16_t(v,&tp->t_state, strocat(f, ".t_state", l),&err);
> +    ptr = tp->t_timer;
> +    visit_start_array(v, (void **)&ptr, strocat(f, ".t_timer", l), TCPT_NTIMERS, sizeof(*ptr),&err);
> +    for (i = 0; i<  TCPT_NTIMERS; ++i) {
> +        visit_type_int16_t(v,&ptr[i], NULL,&err);
> +    }
> +    visit_end_array(v,&err);
> +    visit_type_int16_t(v,&tp->t_rxtshift, strocat(f, ".t_rxtshift", l),&err);


Hrm, you should never concat a name like this.  A better approach would be:

visit_start_struct(v, NULL, f);
visit_type_int16_t(v, &tp->t_rxtshift, "t_rxtshift", &err);
...
visit_end_struct(v, NULL, f);

structs indicate hierarchy.  They don't have to correspond to real types.

Regards,

Anthony Liguori

> +    visit_type_int16_t(v,&tp->t_rxtcur, strocat(f, ".t_rxtcur", l),&err);
> +    visit_type_int16_t(v,&tp->t_dupacks, strocat(f, ".t_dupacks", l),&err);
> +    visit_type_uint16_t(v,&tp->t_maxseg, strocat(f, ".t_maxseg", l),&err);
> +    visit_type_uint8_t(v, (uint8_t *)&tp->t_force, strocat(f, ".t_force", l),&err);
> +    visit_type_uint16_t(v,&tp->t_flags, strocat(f, ".t_flags", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_una, strocat(f, ".snd_una", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_nxt, strocat(f, ".snd_nxt", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_up, strocat(f, ".snd_up", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_wl1, strocat(f, ".snd_wl1", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_wl2, strocat(f, ".snd_wl2", l),&err);
> +    visit_type_uint32_t(v,&tp->iss, strocat(f, ".iss", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_wnd, strocat(f, ".snd_wnd", l),&err);
> +    visit_type_uint32_t(v,&tp->rcv_wnd, strocat(f, ".rcv_wnd", l),&err);
> +    visit_type_uint32_t(v,&tp->rcv_nxt, strocat(f, ".rcv_nxt", l),&err);
> +    visit_type_uint32_t(v,&tp->rcv_up, strocat(f, ".rcv_up", l),&err);
> +    visit_type_uint32_t(v,&tp->irs, strocat(f, ".irs", l),&err);
> +    visit_type_uint32_t(v,&tp->rcv_adv, strocat(f, ".rcv_adv", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_max, strocat(f, ".snd_max", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_cwnd, strocat(f, ".snd_cwnd", l),&err);
> +    visit_type_uint32_t(v,&tp->snd_ssthresh, strocat(f, ".snd_ssthresh", l),&err);
> +    visit_type_int16_t(v,&tp->t_idle, strocat(f, ".t_idle", l),&err);
> +    visit_type_int16_t(v,&tp->t_rtt, strocat(f, ".t_rtt", l),&err);
> +    visit_type_uint32_t(v,&tp->t_rtseq, strocat(f, ".t_rtseq", l),&err);
> +    visit_type_int16_t(v,&tp->t_srtt, strocat(f, ".t_srtt", l),&err);
> +    visit_type_int16_t(v,&tp->t_rttvar, strocat(f, ".t_rttvar", l),&err);
> +    visit_type_uint16_t(v,&tp->t_rttmin, strocat(f, ".t_rttmin", l),&err);
> +    visit_type_uint32_t(v,&tp->max_sndwnd, strocat(f, ".max_sndwnd", l),&err);
> +    visit_type_uint8_t(v, (uint8_t *)&tp->t_oobflags, strocat(f, ".t_oobflags", l),&err);
> +    visit_type_uint8_t(v, (uint8_t *)&tp->t_iobc, strocat(f, ".t_iobc", l),&err);
> +    visit_type_int16_t(v,&tp->t_softerror, strocat(f, ".t_softerror", l),&err);
> +    visit_type_uint8_t(v,&tp->snd_scale, strocat(f, ".snd_scale", l),&err);
> +    visit_type_uint8_t(v,&tp->rcv_scale, strocat(f, ".rcv_scale", l),&err);
> +    visit_type_uint8_t(v,&tp->request_r_scale, strocat(f, ".request_r_scale", l),&err);
> +    visit_type_uint8_t(v,&tp->requested_s_scale, strocat(f, ".requested_s_scale", l),&err);
> +    visit_type_uint32_t(v,&tp->ts_recent, strocat(f, ".ts_recent", l),&err);
> +    visit_type_uint32_t(v,&tp->ts_recent_age, strocat(f, ".ts_recent_age", l),&err);
> +    visit_type_uint32_t(v,&tp->last_ack_sent, strocat(f, ".last_ack_sent", l),&err);
>   }
>
> -static void slirp_sbuf_save(QEMUFile *f, struct sbuf *sbuf)
> +static void slirp_tcp_save(Visitor *v, struct tcpcb *tp, const char *pfield, Error *err)
>   {
> -    uint32_t off;
> +    slirp_tcp_visit(v, tp, pfield, err);
> +}
>
> -    qemu_put_be32(f, sbuf->sb_cc);
> -    qemu_put_be32(f, sbuf->sb_datalen);
> +static void slirp_sbuf_save(Visitor *v, struct sbuf *sbuf, const char *pfield, Error *err)
> +{
> +    int32_t off;
> +    char f[128];
> +    int i, l = 0;
> +
> +    if (pfield) {
> +        assert(strlen(pfield)<  sizeof(f));
> +        strcpy(f, pfield);
> +        l = strlen(f);
> +    }
> +
> +    visit_type_uint32_t(v,&sbuf->sb_cc, strocat(f, ".sb_cc", l),&err);
> +    visit_type_uint32_t(v,&sbuf->sb_datalen, strocat(f, ".sb_datalen", l),&err);
>       off = (uint32_t)(sbuf->sb_wptr - sbuf->sb_data);
> -    qemu_put_sbe32(f, off);
> +    visit_type_int32_t(v,&off, strocat(f, ".sb_wptr_off", l),&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_t(v,&off, strocat(f, ".sb_rptr_off", l),&err);
> +
> +    visit_start_array(v, (void **)&sbuf->sb_data, strocat(f, ".sb_data", l),
> +                      sbuf->sb_datalen, sizeof(*sbuf->sb_data),&err);
> +    for (i = 0; i<  sbuf->sb_datalen; i++) {
> +        visit_type_uint8_t(v, (uint8_t *)&sbuf->sb_data[i], NULL,&err);
> +    }
> +    visit_end_array(v,&err);
> +}
> +
> +static void slirp_socket_visit(Visitor *v, struct socket *so, const char *pfield, Error *err)
> +{
> +    char f[64];
> +    int l = 0;
> +
> +    if (pfield) {
> +        assert(strlen(pfield)<  sizeof(f));
> +        strcpy(f, pfield);
> +        l = strlen(f);
> +    }
> +    visit_type_int32_t(v,&so->so_urgc, strocat(f, ".so_urgc", l),&err);
> +    visit_type_uint32_t(v,&so->so_faddr.s_addr, strocat(f, ".so_faddr.s_addr", l),&err);
> +    visit_type_uint32_t(v,&so->so_laddr.s_addr, strocat(f, ".so_urgc", l),&err);
> +    visit_type_uint16_t(v,&so->so_fport, strocat(f, ".so_fport", l),&err);
> +    visit_type_uint16_t(v,&so->so_lport, strocat(f, ".so_lport", l),&err);
> +    visit_type_uint8_t(v,&so->so_iptos, strocat(f, ".so_iptos", l),&err);
> +    visit_type_uint8_t(v,&so->so_emu, strocat(f, ".so_emu", l),&err);
> +    visit_type_uint8_t(v,&so->so_type, strocat(f, ".so_type", l),&err);
> +    visit_type_int32_t(v,&so->so_state, strocat(f, ".so_state", l),&err);
>   }
>
> -static void slirp_socket_save(QEMUFile *f, struct socket *so)
> +static void slirp_socket_save(Visitor *v, struct socket *so, const char *pfield, 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);
> +    char f[64];
> +    int l = 0;
> +
> +    if (pfield) {
> +        assert(strlen(pfield)<  sizeof(f));
> +        strcpy(f, pfield);
> +        l = strlen(f);
> +    }
> +
> +    slirp_socket_visit(v, so, f, err);
> +
> +    slirp_sbuf_save(v,&so->so_rcv, strocat(f, ".so_rcv", l), err);
> +    slirp_sbuf_save(v,&so->so_snd, strocat(f, ".so_snd", l), err);
> +    slirp_tcp_save(v, so->so_tcpcb, strocat(f, ".so_tcpcb", l), err);
>   }
>
> -static void slirp_bootp_save(QEMUFile *f, Slirp *slirp)
> +static void slirp_bootp_visit(Visitor *v, Slirp *slirp, const char *pfield, Error *err)
>   {
> -    int i;
> +    int i, j, l;
> +    void *ptr;
> +    char f[64];
> +
> +    if (pfield) {
> +        assert(strlen(pfield)<  sizeof(f));
> +        strcpy(f, pfield);
> +        l = strlen(f);
> +    }
>
> +    ptr = slirp->bootp_clients;
> +    visit_start_array(v,&ptr, strocat(f, ".bootp_clients", l), NB_BOOTP_CLIENTS, 8,&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_type_uint16_t(v,&slirp->bootp_clients[i].allocated, "allocated",&err);
> +        ptr = slirp->bootp_clients[i].macaddr;
> +        visit_start_array(v,&ptr, "macaddr", 6, 1,&err);
> +        for (j = 0; j<  6; j++) {
> +            visit_type_uint8_t(v, (uint8_t *)&slirp->bootp_clients[j], NULL,&err);
> +        }
> +        visit_end_array(v,&err);
>       }
> +    visit_end_array(v,&err);
>   }
>
>   static void slirp_state_save(QEMUFile *f, void *opaque)
>   {
>       Slirp *slirp = opaque;
>       struct ex_list *ex_ptr;
> +    int i = 0;
> +    uint8_t padding;
> +    Visitor *v = qemu_file_get_output_visitor(f);
> +    Error *err = NULL;
> +    char id[32];
>
>       for (ex_ptr = slirp->exec_list; ex_ptr; ex_ptr = ex_ptr->ex_next)
>           if (ex_ptr->ex_pty == 3) {
> @@ -970,70 +1048,44 @@ static void slirp_state_save(QEMUFile *f, void *opaque)
>               if (!so)
>                   continue;
>
> -            qemu_put_byte(f, 42);
> -            slirp_socket_save(f, so);
> +            padding = SLIRP_DELIMITER;
> +            visit_type_uint8_t(v,&padding, "padding",&err);
> +            slirp_socket_save(v, so, strocat(id, "so", strlen(id)), err);
>           }
> -    qemu_put_byte(f, 0);
> +    padding = 0;
> +    visit_type_uint8_t(v,&padding, "padding",&err);
> +
> +    visit_type_uint16_t(v,&slirp->ip_id, "slirp.ip_id",&err);
>
> -    qemu_put_be16(f, slirp->ip_id);
> +    slirp_bootp_visit(v, slirp, "slirp", err);
>
> -    slirp_bootp_save(f, slirp);
> +    if (err) {
> +        error_report("error saving slirp state: %s", error_get_pretty(err));
> +        error_free(err);
> +    }
>   }
>
> -static void slirp_tcp_load(QEMUFile *f, struct tcpcb *tp)
> +static void slirp_tcp_load(Visitor *v, struct tcpcb *tp, const char *pfield, Error *err)
>   {
> -    int i;
> -
> -    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);
> +    slirp_tcp_visit(v, tp, pfield, err);
> +
>       tcp_template(tp);
>   }
>
> -static int slirp_sbuf_load(QEMUFile *f, struct sbuf *sbuf)
> +static int slirp_sbuf_load(Visitor *v, struct sbuf *sbuf, const char *pfield, Error *err)
>   {
>       uint32_t off, sb_cc, sb_datalen;
> +    int l = 0, i;
> +    char f[64];
> +
> +    if (pfield) {
> +        assert(strlen(pfield)<  sizeof(f));
> +        strcpy(f, pfield);
> +        l = strlen(f);
> +    }
>
> -    sb_cc = qemu_get_be32(f);
> -    sb_datalen = qemu_get_be32(f);
> +    visit_type_uint32_t(v,&sb_cc, strocat(f, ".sb_cc", l),&err);
> +    visit_type_uint32_t(v,&sb_datalen, strocat(f, ".sb_datalen", l),&err);
>
>       sbreserve(sbuf, sb_datalen);
>
> @@ -1042,61 +1094,68 @@ static int slirp_sbuf_load(QEMUFile *f, struct sbuf *sbuf)
>
>       sbuf->sb_cc = sb_cc;
>
> -    off = qemu_get_sbe32(f);
> +    visit_type_uint32_t(v,&off, strocat(f, ".sb_wptr_off", l),&err);
>       sbuf->sb_wptr = sbuf->sb_data + off;
> -    off = qemu_get_sbe32(f);
> +    visit_type_uint32_t(v,&off, strocat(f, ".sb_rptr_off", l),&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, strocat(f, ".sb_data", l),
> +                      sbuf->sb_datalen, sizeof(*sbuf->sb_data),&err);
> +    for (i = 0; i<  sbuf->sb_datalen; i++) {
> +        visit_type_uint8_t(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, const char *pfield, Error *err)
>   {
> +    char f[64];
> +    int l = 0;
> +
> +    if (pfield) {
> +        assert(strlen(pfield)<  sizeof(f));
> +        strcpy(f, pfield);
> +        l = strlen(f);
> +    }
> +
> +    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)
> +    slirp_socket_visit(v, so, pfield, err);
> +
> +    if (slirp_sbuf_load(v,&so->so_rcv, strocat(f, ".so_rcv", l), err)<  0)
>           return -ENOMEM;
> -    if (slirp_sbuf_load(f,&so->so_snd)<  0)
> +    if (slirp_sbuf_load(v,&so->so_snd, strocat(f, ".so_snd", l), err)<  0)
>           return -ENOMEM;
> -    slirp_tcp_load(f, so->so_tcpcb);
> +    slirp_tcp_load(v, so->so_tcpcb, strocat(f, ".so_tcpcb", l), err);
>
>       return 0;
>   }
>
> -static void slirp_bootp_load(QEMUFile *f, Slirp *slirp)
> -{
> -    int i;
> -
> -    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);
> -    }
> -}
> -
>   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_input_visitor(f);
> +    Error *err = NULL;
> +    char id[32];
> +    uint8_t padding, i = 0;
>
> -    while (qemu_get_byte(f)) {
> +    visit_type_uint8_t(v,&padding, "padding",&err);
> +
> +    while (padding == SLIRP_DELIMITER) {
>           int ret;
>           struct socket *so = socreate(slirp);
>
>           if (!so)
>               return -ENOMEM;
>
> -        ret = slirp_socket_load(f, so);
> +        sprintf(id, "slirp[%d]", i++);
> +
> +        ret = slirp_socket_load(v, so, strocat(id, "so", strlen(id)), err);
>
>           if (ret<  0)
>               return ret;
> @@ -1118,12 +1177,19 @@ static int slirp_state_load(QEMUFile *f, void *opaque, int version_id)
>           so->extra = (void *)ex_ptr->ex_exec;
>       }
>
> +
>       if (version_id>= 2) {
> -        slirp->ip_id = qemu_get_be16(f);
> +        visit_type_uint16_t(v,&slirp->ip_id, "slirp.ip_id",&err);
>       }
>
>       if (version_id>= 3) {
> -        slirp_bootp_load(f, slirp);
> +        slirp_bootp_visit(v, slirp, "slirp", err);
> +    }
> +
> +    if (err) {
> +        error_report("error loading slirp state: %s", error_get_pretty(err));
> +        error_free(err);
> +        return -EINVAL;
>       }
>
>       return 0;

On Fri, 30 Sep 2011 08:39:49 -0500, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 09/19/2011 09:41 AM, Michael Roth wrote:
> > 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<mdroth@linux.vnet.ibm.com>
> > ---
> >   slirp/slirp.c |  366 +++++++++++++++++++++++++++++++++-----------------------
> >   1 files changed, 216 insertions(+), 150 deletions(-)
> >
> > diff --git a/slirp/slirp.c b/slirp/slirp.c
> > index 19d69eb..8783626 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,96 +874,171 @@ 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, const char *pfield, Error *err)
> >   {
> > -    int i;
> > -
> > -    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);
> > +    char f[128];
> > +    int i, l = 0;
> > +    int16_t *ptr;
> > +
> > +    if (pfield) {
> > +        assert(strlen(pfield)<  sizeof(f));
> > +        strcpy(f, pfield);
> > +        l = strlen(pfield);
> > +    }
> > +
> > +    visit_type_int16_t(v,&tp->t_state, strocat(f, ".t_state", l),&err);
> > +    ptr = tp->t_timer;
> > +    visit_start_array(v, (void **)&ptr, strocat(f, ".t_timer", l), TCPT_NTIMERS, sizeof(*ptr),&err);
> > +    for (i = 0; i<  TCPT_NTIMERS; ++i) {
> > +        visit_type_int16_t(v,&ptr[i], NULL,&err);
> > +    }
> > +    visit_end_array(v,&err);
> > +    visit_type_int16_t(v,&tp->t_rxtshift, strocat(f, ".t_rxtshift", l),&err);
> 
> 
> Hrm, you should never concat a name like this.  A better approach would be:
> 
> visit_start_struct(v, NULL, f);
> visit_type_int16_t(v, &tp->t_rxtshift, "t_rxtshift", &err);
> ...
> visit_end_struct(v, NULL, f);
> 
> structs indicate hierarchy.  They don't have to correspond to real types.

Yeah, I thought a lot about using visit_start_struct purely for namespacing,
but wasn't sure if that was too hackish. I like it a whole lot better than
manipulating the field names though... I'll rework it and see.

> 
> Regards,
> 
> Anthony Liguori
> 
> > +    visit_type_int16_t(v,&tp->t_rxtcur, strocat(f, ".t_rxtcur", l),&err);
> > +    visit_type_int16_t(v,&tp->t_dupacks, strocat(f, ".t_dupacks", l),&err);
> > +    visit_type_uint16_t(v,&tp->t_maxseg, strocat(f, ".t_maxseg", l),&err);
> > +    visit_type_uint8_t(v, (uint8_t *)&tp->t_force, strocat(f, ".t_force", l),&err);
> > +    visit_type_uint16_t(v,&tp->t_flags, strocat(f, ".t_flags", l),&err);
> > +    visit_type_uint32_t(v,&tp->snd_una, strocat(f, ".snd_una", l),&err);
> > +    visit_type_uint32_t(v,&tp->snd_nxt, strocat(f, ".snd_nxt", l),&err);
> > +    visit_type_uint32_t(v,&tp->snd_up, strocat(f, ".snd_up", l),&err);
> > +    visit_type_uint32_t(v,&tp->snd_wl1, strocat(f, ".snd_wl1", l),&err);
> > +    visit_type_uint32_t(v,&tp->snd_wl2, strocat(f, ".snd_wl2", l),&err);
> > +    visit_type_uint32_t(v,&tp->iss, strocat(f, ".iss", l),&err);
> > +    visit_type_uint32_t(v,&tp->snd_wnd, strocat(f, ".snd_wnd", l),&err);
> > +    visit_type_uint32_t(v,&tp->rcv_wnd, strocat(f, ".rcv_wnd", l),&err);
> > +    visit_type_uint32_t(v,&tp->rcv_nxt, strocat(f, ".rcv_nxt", l),&err);
> > +    visit_type_uint32_t(v,&tp->rcv_up, strocat(f, ".rcv_up", l),&err);
> > +    visit_type_uint32_t(v,&tp->irs, strocat(f, ".irs", l),&err);
> > +    visit_type_uint32_t(v,&tp->rcv_adv, strocat(f, ".rcv_adv", l),&err);
> > +    visit_type_uint32_t(v,&tp->snd_max, strocat(f, ".snd_max", l),&err);
> > +    visit_type_uint32_t(v,&tp->snd_cwnd, strocat(f, ".snd_cwnd", l),&err);
> > +    visit_type_uint32_t(v,&tp->snd_ssthresh, strocat(f, ".snd_ssthresh", l),&err);
> > +    visit_type_int16_t(v,&tp->t_idle, strocat(f, ".t_idle", l),&err);
> > +    visit_type_int16_t(v,&tp->t_rtt, strocat(f, ".t_rtt", l),&err);
> > +    visit_type_uint32_t(v,&tp->t_rtseq, strocat(f, ".t_rtseq", l),&err);
> > +    visit_type_int16_t(v,&tp->t_srtt, strocat(f, ".t_srtt", l),&err);
> > +    visit_type_int16_t(v,&tp->t_rttvar, strocat(f, ".t_rttvar", l),&err);
> > +    visit_type_uint16_t(v,&tp->t_rttmin, strocat(f, ".t_rttmin", l),&err);
> > +    visit_type_uint32_t(v,&tp->max_sndwnd, strocat(f, ".max_sndwnd", l),&err);
> > +    visit_type_uint8_t(v, (uint8_t *)&tp->t_oobflags, strocat(f, ".t_oobflags", l),&err);
> > +    visit_type_uint8_t(v, (uint8_t *)&tp->t_iobc, strocat(f, ".t_iobc", l),&err);
> > +    visit_type_int16_t(v,&tp->t_softerror, strocat(f, ".t_softerror", l),&err);
> > +    visit_type_uint8_t(v,&tp->snd_scale, strocat(f, ".snd_scale", l),&err);
> > +    visit_type_uint8_t(v,&tp->rcv_scale, strocat(f, ".rcv_scale", l),&err);
> > +    visit_type_uint8_t(v,&tp->request_r_scale, strocat(f, ".request_r_scale", l),&err);
> > +    visit_type_uint8_t(v,&tp->requested_s_scale, strocat(f, ".requested_s_scale", l),&err);
> > +    visit_type_uint32_t(v,&tp->ts_recent, strocat(f, ".ts_recent", l),&err);
> > +    visit_type_uint32_t(v,&tp->ts_recent_age, strocat(f, ".ts_recent_age", l),&err);
> > +    visit_type_uint32_t(v,&tp->last_ack_sent, strocat(f, ".last_ack_sent", l),&err);
> >   }
> >
> > -static void slirp_sbuf_save(QEMUFile *f, struct sbuf *sbuf)
> > +static void slirp_tcp_save(Visitor *v, struct tcpcb *tp, const char *pfield, Error *err)
> >   {
> > -    uint32_t off;
> > +    slirp_tcp_visit(v, tp, pfield, err);
> > +}
> >
> > -    qemu_put_be32(f, sbuf->sb_cc);
> > -    qemu_put_be32(f, sbuf->sb_datalen);
> > +static void slirp_sbuf_save(Visitor *v, struct sbuf *sbuf, const char *pfield, Error *err)
> > +{
> > +    int32_t off;
> > +    char f[128];
> > +    int i, l = 0;
> > +
> > +    if (pfield) {
> > +        assert(strlen(pfield)<  sizeof(f));
> > +        strcpy(f, pfield);
> > +        l = strlen(f);
> > +    }
> > +
> > +    visit_type_uint32_t(v,&sbuf->sb_cc, strocat(f, ".sb_cc", l),&err);
> > +    visit_type_uint32_t(v,&sbuf->sb_datalen, strocat(f, ".sb_datalen", l),&err);
> >       off = (uint32_t)(sbuf->sb_wptr - sbuf->sb_data);
> > -    qemu_put_sbe32(f, off);
> > +    visit_type_int32_t(v,&off, strocat(f, ".sb_wptr_off", l),&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_t(v,&off, strocat(f, ".sb_rptr_off", l),&err);
> > +
> > +    visit_start_array(v, (void **)&sbuf->sb_data, strocat(f, ".sb_data", l),
> > +                      sbuf->sb_datalen, sizeof(*sbuf->sb_data),&err);
> > +    for (i = 0; i<  sbuf->sb_datalen; i++) {
> > +        visit_type_uint8_t(v, (uint8_t *)&sbuf->sb_data[i], NULL,&err);
> > +    }
> > +    visit_end_array(v,&err);
> > +}
> > +
> > +static void slirp_socket_visit(Visitor *v, struct socket *so, const char *pfield, Error *err)
> > +{
> > +    char f[64];
> > +    int l = 0;
> > +
> > +    if (pfield) {
> > +        assert(strlen(pfield)<  sizeof(f));
> > +        strcpy(f, pfield);
> > +        l = strlen(f);
> > +    }
> > +    visit_type_int32_t(v,&so->so_urgc, strocat(f, ".so_urgc", l),&err);
> > +    visit_type_uint32_t(v,&so->so_faddr.s_addr, strocat(f, ".so_faddr.s_addr", l),&err);
> > +    visit_type_uint32_t(v,&so->so_laddr.s_addr, strocat(f, ".so_urgc", l),&err);
> > +    visit_type_uint16_t(v,&so->so_fport, strocat(f, ".so_fport", l),&err);
> > +    visit_type_uint16_t(v,&so->so_lport, strocat(f, ".so_lport", l),&err);
> > +    visit_type_uint8_t(v,&so->so_iptos, strocat(f, ".so_iptos", l),&err);
> > +    visit_type_uint8_t(v,&so->so_emu, strocat(f, ".so_emu", l),&err);
> > +    visit_type_uint8_t(v,&so->so_type, strocat(f, ".so_type", l),&err);
> > +    visit_type_int32_t(v,&so->so_state, strocat(f, ".so_state", l),&err);
> >   }
> >
> > -static void slirp_socket_save(QEMUFile *f, struct socket *so)
> > +static void slirp_socket_save(Visitor *v, struct socket *so, const char *pfield, 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);
> > +    char f[64];
> > +    int l = 0;
> > +
> > +    if (pfield) {
> > +        assert(strlen(pfield)<  sizeof(f));
> > +        strcpy(f, pfield);
> > +        l = strlen(f);
> > +    }
> > +
> > +    slirp_socket_visit(v, so, f, err);
> > +
> > +    slirp_sbuf_save(v,&so->so_rcv, strocat(f, ".so_rcv", l), err);
> > +    slirp_sbuf_save(v,&so->so_snd, strocat(f, ".so_snd", l), err);
> > +    slirp_tcp_save(v, so->so_tcpcb, strocat(f, ".so_tcpcb", l), err);
> >   }
> >
> > -static void slirp_bootp_save(QEMUFile *f, Slirp *slirp)
> > +static void slirp_bootp_visit(Visitor *v, Slirp *slirp, const char *pfield, Error *err)
> >   {
> > -    int i;
> > +    int i, j, l;
> > +    void *ptr;
> > +    char f[64];
> > +
> > +    if (pfield) {
> > +        assert(strlen(pfield)<  sizeof(f));
> > +        strcpy(f, pfield);
> > +        l = strlen(f);
> > +    }
> >
> > +    ptr = slirp->bootp_clients;
> > +    visit_start_array(v,&ptr, strocat(f, ".bootp_clients", l), NB_BOOTP_CLIENTS, 8,&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_type_uint16_t(v,&slirp->bootp_clients[i].allocated, "allocated",&err);
> > +        ptr = slirp->bootp_clients[i].macaddr;
> > +        visit_start_array(v,&ptr, "macaddr", 6, 1,&err);
> > +        for (j = 0; j<  6; j++) {
> > +            visit_type_uint8_t(v, (uint8_t *)&slirp->bootp_clients[j], NULL,&err);
> > +        }
> > +        visit_end_array(v,&err);
> >       }
> > +    visit_end_array(v,&err);
> >   }
> >
> >   static void slirp_state_save(QEMUFile *f, void *opaque)
> >   {
> >       Slirp *slirp = opaque;
> >       struct ex_list *ex_ptr;
> > +    int i = 0;
> > +    uint8_t padding;
> > +    Visitor *v = qemu_file_get_output_visitor(f);
> > +    Error *err = NULL;
> > +    char id[32];
> >
> >       for (ex_ptr = slirp->exec_list; ex_ptr; ex_ptr = ex_ptr->ex_next)
> >           if (ex_ptr->ex_pty == 3) {
> > @@ -970,70 +1048,44 @@ static void slirp_state_save(QEMUFile *f, void *opaque)
> >               if (!so)
> >                   continue;
> >
> > -            qemu_put_byte(f, 42);
> > -            slirp_socket_save(f, so);
> > +            padding = SLIRP_DELIMITER;
> > +            visit_type_uint8_t(v,&padding, "padding",&err);
> > +            slirp_socket_save(v, so, strocat(id, "so", strlen(id)), err);
> >           }
> > -    qemu_put_byte(f, 0);
> > +    padding = 0;
> > +    visit_type_uint8_t(v,&padding, "padding",&err);
> > +
> > +    visit_type_uint16_t(v,&slirp->ip_id, "slirp.ip_id",&err);
> >
> > -    qemu_put_be16(f, slirp->ip_id);
> > +    slirp_bootp_visit(v, slirp, "slirp", err);
> >
> > -    slirp_bootp_save(f, slirp);
> > +    if (err) {
> > +        error_report("error saving slirp state: %s", error_get_pretty(err));
> > +        error_free(err);
> > +    }
> >   }
> >
> > -static void slirp_tcp_load(QEMUFile *f, struct tcpcb *tp)
> > +static void slirp_tcp_load(Visitor *v, struct tcpcb *tp, const char *pfield, Error *err)
> >   {
> > -    int i;
> > -
> > -    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);
> > +    slirp_tcp_visit(v, tp, pfield, err);
> > +
> >       tcp_template(tp);
> >   }
> >
> > -static int slirp_sbuf_load(QEMUFile *f, struct sbuf *sbuf)
> > +static int slirp_sbuf_load(Visitor *v, struct sbuf *sbuf, const char *pfield, Error *err)
> >   {
> >       uint32_t off, sb_cc, sb_datalen;
> > +    int l = 0, i;
> > +    char f[64];
> > +
> > +    if (pfield) {
> > +        assert(strlen(pfield)<  sizeof(f));
> > +        strcpy(f, pfield);
> > +        l = strlen(f);
> > +    }
> >
> > -    sb_cc = qemu_get_be32(f);
> > -    sb_datalen = qemu_get_be32(f);
> > +    visit_type_uint32_t(v,&sb_cc, strocat(f, ".sb_cc", l),&err);
> > +    visit_type_uint32_t(v,&sb_datalen, strocat(f, ".sb_datalen", l),&err);
> >
> >       sbreserve(sbuf, sb_datalen);
> >
> > @@ -1042,61 +1094,68 @@ static int slirp_sbuf_load(QEMUFile *f, struct sbuf *sbuf)
> >
> >       sbuf->sb_cc = sb_cc;
> >
> > -    off = qemu_get_sbe32(f);
> > +    visit_type_uint32_t(v,&off, strocat(f, ".sb_wptr_off", l),&err);
> >       sbuf->sb_wptr = sbuf->sb_data + off;
> > -    off = qemu_get_sbe32(f);
> > +    visit_type_uint32_t(v,&off, strocat(f, ".sb_rptr_off", l),&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, strocat(f, ".sb_data", l),
> > +                      sbuf->sb_datalen, sizeof(*sbuf->sb_data),&err);
> > +    for (i = 0; i<  sbuf->sb_datalen; i++) {
> > +        visit_type_uint8_t(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, const char *pfield, Error *err)
> >   {
> > +    char f[64];
> > +    int l = 0;
> > +
> > +    if (pfield) {
> > +        assert(strlen(pfield)<  sizeof(f));
> > +        strcpy(f, pfield);
> > +        l = strlen(f);
> > +    }
> > +
> > +    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)
> > +    slirp_socket_visit(v, so, pfield, err);
> > +
> > +    if (slirp_sbuf_load(v,&so->so_rcv, strocat(f, ".so_rcv", l), err)<  0)
> >           return -ENOMEM;
> > -    if (slirp_sbuf_load(f,&so->so_snd)<  0)
> > +    if (slirp_sbuf_load(v,&so->so_snd, strocat(f, ".so_snd", l), err)<  0)
> >           return -ENOMEM;
> > -    slirp_tcp_load(f, so->so_tcpcb);
> > +    slirp_tcp_load(v, so->so_tcpcb, strocat(f, ".so_tcpcb", l), err);
> >
> >       return 0;
> >   }
> >
> > -static void slirp_bootp_load(QEMUFile *f, Slirp *slirp)
> > -{
> > -    int i;
> > -
> > -    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);
> > -    }
> > -}
> > -
> >   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_input_visitor(f);
> > +    Error *err = NULL;
> > +    char id[32];
> > +    uint8_t padding, i = 0;
> >
> > -    while (qemu_get_byte(f)) {
> > +    visit_type_uint8_t(v,&padding, "padding",&err);
> > +
> > +    while (padding == SLIRP_DELIMITER) {
> >           int ret;
> >           struct socket *so = socreate(slirp);
> >
> >           if (!so)
> >               return -ENOMEM;
> >
> > -        ret = slirp_socket_load(f, so);
> > +        sprintf(id, "slirp[%d]", i++);
> > +
> > +        ret = slirp_socket_load(v, so, strocat(id, "so", strlen(id)), err);
> >
> >           if (ret<  0)
> >               return ret;
> > @@ -1118,12 +1177,19 @@ static int slirp_state_load(QEMUFile *f, void *opaque, int version_id)
> >           so->extra = (void *)ex_ptr->ex_exec;
> >       }
> >
> > +
> >       if (version_id>= 2) {
> > -        slirp->ip_id = qemu_get_be16(f);
> > +        visit_type_uint16_t(v,&slirp->ip_id, "slirp.ip_id",&err);
> >       }
> >
> >       if (version_id>= 3) {
> > -        slirp_bootp_load(f, slirp);
> > +        slirp_bootp_visit(v, slirp, "slirp", err);
> > +    }
> > +
> > +    if (err) {
> > +        error_report("error loading slirp state: %s", error_get_pretty(err));
> > +        error_free(err);
> > +        return -EINVAL;
> >       }
> >
> >       return 0;
>