[PATCH 1/2] vsh: Make vshInitDebug return int instead of void

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

 



Well, the reason behind this change is that if the function is extended in some
way that e.g. would involve allocation we do not have a way of telling it to
the caller. More specifically, vshInitDebug only relies on some hardcoded
environment variables (by a mistake) that aren't documented anywhere so neither
virsh's nor virt-admin's documented environment variables take effect. One
possible solution would be duplicate the code for each CLI client or leave the
method be generic and provide means that it could figure out, which client
called it, thus initializing the proper environment variables but that could
involve operations that might as well fail in certain circumstances and the
caller should know that an error occurred.

Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx>
---
 tools/vsh.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 9ac4f21..65bd9a4 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2966,7 +2966,7 @@ vshReadline(vshControl *ctl, const char *prompt)
 /*
  * Initialize debug settings.
  */
-static void
+static int
 vshInitDebug(vshControl *ctl)
 {
     const char *debugEnv;
@@ -2994,6 +2994,8 @@ vshInitDebug(vshControl *ctl)
             vshOpenLogFile(ctl);
         }
     }
+
+    return 0;
 }
 
 
@@ -3016,9 +3018,9 @@ vshInit(vshControl *ctl, const vshCmdGrp *groups, const vshCmdDef *set)
 
     cmdGroups = groups;
     cmdSet = set;
-    vshInitDebug(ctl);
 
-    if (ctl->imode && vshReadlineInit(ctl) < 0)
+    if (vshInitDebug(ctl) < 0 ||
+        (ctl->imode && vshReadlineInit(ctl) < 0))
         return false;
 
     return true;
@@ -3033,7 +3035,8 @@ vshInitReload(vshControl *ctl)
         return false;
     }
 
-    vshInitDebug(ctl);
+    if (vshInitDebug(ctl) < 0)
+        return false;
 
     if (ctl->imode)
         vshReadlineDeinit(ctl);
-- 
2.5.5

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[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]