[PATCH] tools/virsh-pool: refactor smaller functions

Kristina Hanicova posted 1 patch 2 years, 7 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/500bda686590866c15f3178dd85eae8127a8093b.1631715926.git.khanicov@redhat.com
tools/virsh-pool.c | 68 ++++++++++++++++++++--------------------------
1 file changed, 30 insertions(+), 38 deletions(-)
[PATCH] tools/virsh-pool: refactor smaller functions
Posted by Kristina Hanicova 2 years, 7 months ago
I think these functions look much more readable with just simple
if conditions.

Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
---
 tools/virsh-pool.c | 68 ++++++++++++++++++++--------------------------
 1 file changed, 30 insertions(+), 38 deletions(-)

diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index 1a2ab8cc53..c65e8163a6 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -269,7 +269,6 @@ cmdPoolCreate(vshControl *ctl, const vshCmd *cmd)
 {
     virStoragePoolPtr pool;
     const char *from = NULL;
-    bool ret = true;
     g_autofree char *buffer = NULL;
     bool build;
     bool overwrite;
@@ -297,17 +296,15 @@ cmdPoolCreate(vshControl *ctl, const vshCmd *cmd)
     if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
         return false;
 
-    pool = virStoragePoolCreateXML(priv->conn, buffer, flags);
-
-    if (pool != NULL) {
-        vshPrintExtra(ctl, _("Pool %s created from %s\n"),
-                      virStoragePoolGetName(pool), from);
-        virStoragePoolFree(pool);
-    } else {
+    if (!(pool = virStoragePoolCreateXML(priv->conn, buffer, flags))) {
         vshError(ctl, _("Failed to create pool from %s"), from);
-        ret = false;
+        return false;
     }
-    return ret;
+
+    vshPrintExtra(ctl, _("Pool %s created from %s\n"),
+                  virStoragePoolGetName(pool), from);
+    virStoragePoolFree(pool);
+    return true;
 }
 
 static const vshCmdOptDef opts_pool_define_as[] = {
@@ -490,17 +487,16 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
 
     if (printXML) {
         vshPrint(ctl, "%s", xml);
-    } else {
-        pool = virStoragePoolCreateXML(priv->conn, xml, flags);
+        return true;
+    }
 
-        if (pool != NULL) {
-            vshPrintExtra(ctl, _("Pool %s created\n"), name);
-            virStoragePoolFree(pool);
-        } else {
-            vshError(ctl, _("Failed to create pool %s"), name);
-            return false;
-        }
+    if (!(pool = virStoragePoolCreateXML(priv->conn, xml, flags))) {
+        vshError(ctl, _("Failed to create pool %s"), name);
+        return false;
     }
+
+    vshPrintExtra(ctl, _("Pool %s created\n"), name);
+    virStoragePoolFree(pool);
     return true;
 }
 
@@ -532,7 +528,6 @@ cmdPoolDefine(vshControl *ctl, const vshCmd *cmd)
 {
     virStoragePoolPtr pool;
     const char *from = NULL;
-    bool ret = true;
     g_autofree char *buffer = NULL;
     unsigned int flags = 0;
     virshControl *priv = ctl->privData;
@@ -546,17 +541,15 @@ cmdPoolDefine(vshControl *ctl, const vshCmd *cmd)
     if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
         return false;
 
-    pool = virStoragePoolDefineXML(priv->conn, buffer, flags);
-
-    if (pool != NULL) {
-        vshPrintExtra(ctl, _("Pool %s defined from %s\n"),
-                      virStoragePoolGetName(pool), from);
-        virStoragePoolFree(pool);
-    } else {
+    if (!(pool = virStoragePoolDefineXML(priv->conn, buffer, flags))) {
         vshError(ctl, _("Failed to define pool from %s"), from);
-        ret = false;
+        return false;
     }
-    return ret;
+
+    vshPrintExtra(ctl, _("Pool %s defined from %s\n"),
+                  virStoragePoolGetName(pool), from);
+    virStoragePoolFree(pool);
+    return true;
 }
 
 /*
@@ -586,17 +579,16 @@ cmdPoolDefineAs(vshControl *ctl, const vshCmd *cmd)
 
     if (printXML) {
         vshPrint(ctl, "%s", xml);
-    } else {
-        pool = virStoragePoolDefineXML(priv->conn, xml, 0);
+        return true;
+    }
 
-        if (pool != NULL) {
-            vshPrintExtra(ctl, _("Pool %s defined\n"), name);
-            virStoragePoolFree(pool);
-        } else {
-            vshError(ctl, _("Failed to define pool %s"), name);
-            return false;
-        }
+    if (!(pool = virStoragePoolDefineXML(priv->conn, xml, 0))) {
+        vshError(ctl, _("Failed to define pool %s"), name);
+        return false;
     }
+
+    vshPrintExtra(ctl, _("Pool %s defined\n"), name);
+    virStoragePoolFree(pool);
     return true;
 }
 
-- 
2.31.1

Re: [PATCH] tools/virsh-pool: refactor smaller functions
Posted by Michal Prívozník 2 years, 7 months ago
On 9/15/21 4:27 PM, Kristina Hanicova wrote:
> I think these functions look much more readable with just simple
> if conditions.
> 
> Signed-off-by: Kristina Hanicova <khanicov@redhat.com>
> ---
>  tools/virsh-pool.c | 68 ++++++++++++++++++++--------------------------
>  1 file changed, 30 insertions(+), 38 deletions(-)
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal