Re: [PATCH v2 04/13] virsh-interface: Add a static helper virshInterfaceStringHelper

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 11/10/20 10:50 AM, Lin Ma wrote:
> It will be helpful to get the desired string of interface name/mac in a
> consistent way.
>
> Signed-off-by: Lin Ma <lma@xxxxxxxx>
> ---
>   tools/virsh-completer-interface.c | 25 ++++++++++++++++---------
>   tools/virsh-completer-interface.h |  3 +++
>   2 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/tools/virsh-completer-interface.c b/tools/virsh-completer-interface.c
> index 8028db8746..407021485f 100644
> --- a/tools/virsh-completer-interface.c
> +++ b/tools/virsh-completer-interface.c
> @@ -25,16 +25,16 @@
>   #include "virsh.h"
>   #include "virstring.h"
>   
> -char **
> -virshInterfaceNameCompleter(vshControl *ctl,
> -                            const vshCmd *cmd G_GNUC_UNUSED,
> -                            unsigned int flags)
> +static char **
> +virshInterfaceStringHelper(vshControl *ctl,
> +                           const vshCmd *cmd G_GNUC_UNUSED,
> +                           unsigned int flags,
> +                           virInterfaceStringCallback cb)
>   {
>       virshControlPtr priv = ctl->privData;
>       virInterfacePtr *ifaces = NULL;
>       int nifaces = 0;
>       size_t i = 0;
> -    char **ret = NULL;
>       VIR_AUTOSTRINGLIST tmp = NULL;
>   
>       virCheckFlags(VIR_CONNECT_LIST_INTERFACES_ACTIVE |
> @@ -50,15 +50,22 @@ virshInterfaceNameCompleter(vshControl *ctl,
>       tmp = g_new0(char *, nifaces + 1);
>   
>       for (i = 0; i < nifaces; i++) {
> -        const char *name = virInterfaceGetName(ifaces[i]);
> +        const char *name = (cb)(ifaces[i]);
>   
>           tmp[i] = g_strdup(name);
>       }
>   
> -    ret = g_steal_pointer(&tmp);
> -
>       for (i = 0; i < nifaces; i++)
>           virInterfaceFree(ifaces[i]);
>       g_free(ifaces);
> -    return ret;
> +
> +    return g_steal_pointer(&tmp);
> +}
> +
> +char **
> +virshInterfaceNameCompleter(vshControl *ctl,
> +                            const vshCmd *cmd,
> +                            unsigned int flags)
> +{
> +    return virshInterfaceStringHelper(ctl, cmd, flags, virInterfaceGetName);
>   }
> diff --git a/tools/virsh-completer-interface.h b/tools/virsh-completer-interface.h
> index 893dee5a6b..32da01e766 100644
> --- a/tools/virsh-completer-interface.h
> +++ b/tools/virsh-completer-interface.h
> @@ -22,6 +22,9 @@
>   
>   #include "vsh.h"
>   
> +typedef const char *
> +(*virInterfaceStringCallback)(virInterfacePtr iface);
> +

This definition belongs to virsh-completer-interface.c because the 
function using it is static and thus there is no way anybody else can 
call it hence less header pollution.

Michal