Re: [Qemu-devel] [RFC PATCH] net: introduce monitor command to query mactables

[Stefan Hajnoczi]

On Thu, Apr 11, 2013 at 11:11:58PM +0800, Amos Kong wrote:
> +static MacTableInfo *virtio_net_query_mactable(NetClientState *nc)
> +{
> +    VirtIONet *n = qemu_get_nic_opaque(nc);
> +    MacTableInfo *info;
> +    StringList *str_list = NULL;
> +    StringList *entry;
> +    char str[12];
> +    int i;
> +
> +    info = g_malloc0(sizeof(*info));
> +    info->name = g_strdup(nc->name);
> +
> +    info->promisc = n->promisc;
> +    info->has_promisc = true;
> +    info->allmulti = n->allmulti;
> +    info->has_allmulti = true;
> +    info->alluni = n->alluni;
> +    info->has_alluni = true;
> +    info->nomulti = n->nomulti;
> +    info->has_nomulti = true;
> +    info->nouni = n->nouni;
> +    info->has_nouni = true;
> +    info->nobcast = n->nobcast;
> +    info->has_nobcast = true;
> +    info->multi_overflow = n->mac_table.multi_overflow;
> +    info->has_multi_overflow = true;
> +    info->uni_overflow = n->mac_table.uni_overflow;
> +    info->has_uni_overflow = true;
> +
> +    for (i = 0; i < n->mac_table.first_multi; i++) {
> +        info->has_unicast = true;
> +        entry = g_malloc0(sizeof(*entry));
> +        sprintf(str,
> +                "%.2x:%.2x:%.2x:%.2x:%.2x:%.2x",
> +                n->mac_table.macs[i * ETH_ALEN],
> +                n->mac_table.macs[i * ETH_ALEN + 1],
> +                n->mac_table.macs[i * ETH_ALEN + 2],
> +                n->mac_table.macs[i * ETH_ALEN + 3],
> +                n->mac_table.macs[i * ETH_ALEN + 4],
> +                n->mac_table.macs[i * ETH_ALEN + 5]);

Buffer overflow, char str[12], but luckily...

> +        entry->value = g_malloc0(sizeof(String *));
> +        entry->value->str = g_strdup(str);

...these lines can be replaced with g_strdup_printf():
https://developer.gnome.org/glib/2.28/glib-String-Utility-Functions.html#g-strdup-printf

> diff --git a/net/net.c b/net/net.c
> index 7869161..2103e7f 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -964,6 +964,29 @@ void print_net_client(Monitor *mon, NetClientState *nc)
>                     nc->info_str);
>  }
>  
> +MacTableInfoList *qmp_query_mac_table(Error **errp)
> +{
> +    NetClientState *nc;
> +    MacTableInfoList *table_list = NULL;
> +
> +    QTAILQ_FOREACH(nc, &net_clients, next) {
> +        MacTableInfoList *entry;
> +        MacTableInfo *info;
> +
> +        if (nc->info->type != NET_CLIENT_OPTIONS_KIND_NIC) {
> +            continue;
> +        }
> +        if (nc->info->query_mac_table) {
> +            info = nc->info->query_mac_table(nc);
> +            entry = g_malloc0(sizeof(*entry));
> +            entry->value = info;
> +            entry->next = table_list;
> +            table_list = entry;
> +        }
> +    }
> +    return table_list;
> +}

Please add an optional net client name argument so the user can query
just a single NIC.  This saves users from having to parse out a specific
NIC when they just want to query one.

Stefan