[libvirt] [PATCH 04/19] virsh: Rename virshInterfaceNameCompleter to virshInterfaceCompleter

Michal Privoznik mprivozn at redhat.com
Mon Nov 2 19:41:19 UTC 2020


On 11/2/20 9:26 AM, Lin Ma wrote:
> Rename the function virshInterfaceNameCompleter to virshInterfaceCompleter
> to make it a bit more generic.
> The upcoming patch invokes it for mac completion.
> 
> Signed-off-by: Lin Ma <lma at suse.com>
> ---
>   tools/virsh-completer-interface.c | 6 +++---
>   tools/virsh-completer-interface.h | 6 +++---
>   tools/virsh-interface.c           | 2 +-
>   3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/virsh-completer-interface.c b/tools/virsh-completer-interface.c
> index 8028db8746..777bb22b0b 100644
> --- a/tools/virsh-completer-interface.c
> +++ b/tools/virsh-completer-interface.c
> @@ -26,9 +26,9 @@
>   #include "virstring.h"
>   
>   char **
> -virshInterfaceNameCompleter(vshControl *ctl,
> -                            const vshCmd *cmd G_GNUC_UNUSED,
> -                            unsigned int flags)
> +virshInterfaceCompleter(vshControl *ctl,
> +                        const vshCmd *cmd G_GNUC_UNUSED,
> +                        unsigned int flags)

Looking into the future patches, I think we want a different approach. 
In one of future patches you will switch this to return MAC addresses 
too. Fair enough, points for code re-use. But The way it's implemented 
is confusing and in fact wrong (see my review to 08/19). How about:

1) Moving this body into a static helper, which would accept one 
argument more: callback to get get the desired string
2) This virshInterfaceNameCompleter() would then effectively end up a 
single line call of that callback with virInterfaceGetName() passed as 
the callback,
3) New virshInterfaceMac() would be defined with 
virInterfaceGetMACString() passed as the callback.

Michal




More information about the libvir-list mailing list