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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxx>
---
  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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux