Eric Blake <ebl...@redhat.com> writes:
> Instead of modifying the value member of a list element passed as a
> parameter, and open-coding the manipulation of that list, it's nicer
> to just return a freshly allocated value to be prepended to a list
> using QAPI_LIST_PREPEND.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
> hw/net/rocker/rocker_fp.h | 2 +-
> hw/net/rocker/rocker.c | 8 +-------
> hw/net/rocker/rocker_fp.c | 17 ++++++++++-------
> 3 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/hw/net/rocker/rocker_fp.h b/hw/net/rocker/rocker_fp.h
> index dbe1dd329a4b..7ff57aac0180 100644
> --- a/hw/net/rocker/rocker_fp.h
> +++ b/hw/net/rocker/rocker_fp.h
> @@ -28,7 +28,7 @@ int fp_port_eg(FpPort *port, const struct iovec *iov, int
> iovcnt);
>
> char *fp_port_get_name(FpPort *port);
> bool fp_port_get_link_up(FpPort *port);
> -void fp_port_get_info(FpPort *port, RockerPortList *info);
> +RockerPort *fp_port_get_info(FpPort *port);
> void fp_port_get_macaddr(FpPort *port, MACAddr *macaddr);
> void fp_port_set_macaddr(FpPort *port, MACAddr *macaddr);
> uint8_t fp_port_get_learning(FpPort *port);
> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
> index 1af1e6fa2f9b..c53a02315e54 100644
> --- a/hw/net/rocker/rocker.c
> +++ b/hw/net/rocker/rocker.c
> @@ -127,13 +127,7 @@ RockerPortList *qmp_query_rocker_ports(const char *name,
> Error **errp)
> }
>
> for (i = r->fp_ports - 1; i >= 0; i--) {
> - RockerPortList *info = g_malloc0(sizeof(*info));
> - info->value = g_malloc0(sizeof(*info->value));
> - struct fp_port *port = r->fp_port[i];
> -
> - fp_port_get_info(port, info);
> - info->next = list;
> - list = info;
> + QAPI_LIST_PREPEND(list, fp_port_get_info(r->fp_port[i]));
Relies on QAPI_LIST_PREPEND() evaluating its second argument exactly
once. Part of its written contract; okay.
> }
>
> return list;
> diff --git a/hw/net/rocker/rocker_fp.c b/hw/net/rocker/rocker_fp.c
> index 4aa7da79b81d..cbeed65bd5ec 100644
> --- a/hw/net/rocker/rocker_fp.c
> +++ b/hw/net/rocker/rocker_fp.c
> @@ -51,14 +51,17 @@ bool fp_port_get_link_up(FpPort *port)
> return !qemu_get_queue(port->nic)->link_down;
> }
>
> -void fp_port_get_info(FpPort *port, RockerPortList *info)
> +RockerPort *fp_port_get_info(FpPort *port)
> {
> - info->value->name = g_strdup(port->name);
> - info->value->enabled = port->enabled;
> - info->value->link_up = fp_port_get_link_up(port);
> - info->value->speed = port->speed;
> - info->value->duplex = port->duplex;
> - info->value->autoneg = port->autoneg;
> + RockerPort *value = g_malloc0(sizeof(*value));
> +
> + value->name = g_strdup(port->name);
> + value->enabled = port->enabled;
> + value->link_up = fp_port_get_link_up(port);
> + value->speed = port->speed;
> + value->duplex = port->duplex;
> + value->autoneg = port->autoneg;
> + return value;
> }
>
> void fp_port_get_macaddr(FpPort *port, MACAddr *macaddr)
Reviewed-by: Markus Armbruster <arm...@redhat.com>
