[PATCH 2/2] virsh: kill over-engineered asprintf failure recovery

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

 



I noticed this while shortning switch statements via VIR_ENUM.
Basically, the only ways virAsprintf can fail are if we pass a
bogus format string (but we're not THAT bad) or if we run out
of memory (but it already warns on our behalf in that case).
Throw away the cruft that tries too hard to diagnose a printf
failure.

* tools/virsh-volume.c (cmdVolList): Simplify.
* tools/virsh-pool.c (cmdPoolList): Likewise.

Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---
 tools/virsh-pool.c   | 73 +++++++++++++++++-----------------------------------
 tools/virsh-volume.c | 66 ++++++++++++++---------------------------------
 2 files changed, 42 insertions(+), 97 deletions(-)

diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index ec99564..b21b682 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -961,9 +961,8 @@ static bool
 cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
 {
     virStoragePoolInfo info;
-    int ret;
     size_t i;
-    bool functionReturn = false;
+    bool ret = false;
     size_t stringLength = 0, nameStrLength = 0;
     size_t autostartStrLength = 0, persistStrLength = 0;
     size_t stateStrLength = 0, capStrLength = 0;
@@ -1121,29 +1120,20 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
                     double val;
                     const char *unit;

-                    /* Create the capacity output string */
                     val = vshPrettyCapacity(info.capacity, &unit);
-                    ret = virAsprintf(&poolInfoTexts[i].capacity,
-                                      "%.2lf %s", val, unit);
-                    if (ret < 0)
-                        /* An error occurred creating the string, return */
-                        goto asprintf_failure;
+                    if (virAsprintf(&poolInfoTexts[i].capacity,
+                                    "%.2lf %s", val, unit) < 0)
+                        goto cleanup;

-                    /* Create the allocation output string */
                     val = vshPrettyCapacity(info.allocation, &unit);
-                    ret = virAsprintf(&poolInfoTexts[i].allocation,
-                                      "%.2lf %s", val, unit);
-                    if (ret < 0)
-                        /* An error occurred creating the string, return */
-                        goto asprintf_failure;
+                    if (virAsprintf(&poolInfoTexts[i].allocation,
+                                    "%.2lf %s", val, unit) < 0)
+                        goto cleanup;

-                    /* Create the available space output string */
                     val = vshPrettyCapacity(info.available, &unit);
-                    ret = virAsprintf(&poolInfoTexts[i].available,
-                                      "%.2lf %s", val, unit);
-                    if (ret < 0)
-                        /* An error occurred creating the string, return */
-                        goto asprintf_failure;
+                    if (virAsprintf(&poolInfoTexts[i].available,
+                                    "%.2lf %s", val, unit) < 0)
+                        goto cleanup;
                 } else {
                     /* Capacity related information isn't available */
                     poolInfoTexts[i].capacity = vshStrdup(ctl, _("-"));
@@ -1213,7 +1203,7 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
         }

         /* Cleanup and return */
-        functionReturn = true;
+        ret = true;
         goto cleanup;
     }

@@ -1273,19 +1263,16 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     /* Create the output template.  Each column is sized according to
      * the longest string.
      */
-    ret = virAsprintf(&outputStr,
-              " %%-%lus  %%-%lus  %%-%lus  %%-%lus  %%%lus  %%%lus  %%%lus\n",
-              (unsigned long) nameStrLength,
-              (unsigned long) stateStrLength,
-              (unsigned long) autostartStrLength,
-              (unsigned long) persistStrLength,
-              (unsigned long) capStrLength,
-              (unsigned long) allocStrLength,
-              (unsigned long) availStrLength);
-    if (ret < 0) {
-        /* An error occurred creating the string, return */
-        goto asprintf_failure;
-    }
+    if (virAsprintf(&outputStr,
+                    " %%-%lus  %%-%lus  %%-%lus  %%-%lus  %%%lus  %%%lus  %%%lus\n",
+                    (unsigned long) nameStrLength,
+                    (unsigned long) stateStrLength,
+                    (unsigned long) autostartStrLength,
+                    (unsigned long) persistStrLength,
+                    (unsigned long) capStrLength,
+                    (unsigned long) allocStrLength,
+                    (unsigned long) availStrLength) < 0)
+        goto cleanup;

     /* Display the header */
     vshPrint(ctl, outputStr, _("Name"), _("State"), _("Autostart"),
@@ -1310,21 +1297,7 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     }

     /* Cleanup and return */
-    functionReturn = true;
-    goto cleanup;
-
-asprintf_failure:
-    /* Display an appropriate error message then cleanup and return */
-    switch (errno) {
-    case ENOMEM:
-        /* Couldn't allocate memory */
-        vshError(ctl, "%s", _("Out of memory"));
-        break;
-    default:
-        /* Some other error */
-        vshError(ctl, _("virAsprintf failed (errno %d)"), errno);
-    }
-    functionReturn = false;
+    ret = true;

 cleanup:
     VIR_FREE(outputStr);
@@ -1341,7 +1314,7 @@ cleanup:
     VIR_FREE(poolInfoTexts);

     vshStoragePoolListFree(list);
-    return functionReturn;
+    return ret;
 }

 /*
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 839a17c..898b34b 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -1332,8 +1332,7 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     double val;
     bool details = vshCommandOptBool(cmd, "details");
     size_t i;
-    int ret;
-    bool functionReturn = false;
+    bool ret = false;
     int stringLength = 0;
     size_t allocStrLength = 0, capStrLength = 0;
     size_t nameStrLength = 0, pathStrLength = 0;
@@ -1382,23 +1381,15 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
                 volInfoTexts[i].type = vshStrdup(ctl,
                                                  vshVolumeTypeToString(volumeInfo.type));

-                /* Create the capacity output string */
                 val = vshPrettyCapacity(volumeInfo.capacity, &unit);
-                ret = virAsprintf(&volInfoTexts[i].capacity,
-                                  "%.2lf %s", val, unit);
-                if (ret < 0) {
-                    /* An error occurred creating the string, return */
-                    goto asprintf_failure;
-                }
-
-                /* Create the allocation output string */
+                if (virAsprintf(&volInfoTexts[i].capacity,
+                                "%.2lf %s", val, unit) < 0)
+                    goto cleanup;
+
                 val = vshPrettyCapacity(volumeInfo.allocation, &unit);
-                ret = virAsprintf(&volInfoTexts[i].allocation,
-                                  "%.2lf %s", val, unit);
-                if (ret < 0) {
-                    /* An error occurred creating the string, return */
-                    goto asprintf_failure;
-                }
+                if (virAsprintf(&volInfoTexts[i].allocation,
+                                "%.2lf %s", val, unit) < 0)
+                    goto cleanup;
             }

             /* Remember the largest length for each output string.
@@ -1450,7 +1441,7 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
         }

         /* Cleanup and return */
-        functionReturn = true;
+        ret = true;
         goto cleanup;
     }

@@ -1493,18 +1484,14 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     vshDebug(ctl, VSH_ERR_DEBUG,
              "Longest allocation string = %zu chars\n", allocStrLength);

-    /* Create the output template */
-    ret = virAsprintf(&outputStr,
-                      " %%-%lus  %%-%lus  %%-%lus  %%%lus  %%%lus\n",
-                      (unsigned long) nameStrLength,
-                      (unsigned long) pathStrLength,
-                      (unsigned long) typeStrLength,
-                      (unsigned long) capStrLength,
-                      (unsigned long) allocStrLength);
-    if (ret < 0) {
-        /* An error occurred creating the string, return */
-        goto asprintf_failure;
-    }
+    if (virAsprintf(&outputStr,
+                    " %%-%lus  %%-%lus  %%-%lus  %%%lus  %%%lus\n",
+                    (unsigned long) nameStrLength,
+                    (unsigned long) pathStrLength,
+                    (unsigned long) typeStrLength,
+                    (unsigned long) capStrLength,
+                    (unsigned long) allocStrLength) < 0)
+        goto cleanup;

     /* Display the header */
     vshPrint(ctl, outputStr, _("Name"), _("Path"), _("Type"),
@@ -1526,22 +1513,7 @@ cmdVolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
     }

     /* Cleanup and return */
-    functionReturn = true;
-    goto cleanup;
-
-asprintf_failure:
-
-    /* Display an appropriate error message then cleanup and return */
-    switch (errno) {
-    case ENOMEM:
-        /* Couldn't allocate memory */
-        vshError(ctl, "%s", _("Out of memory"));
-        break;
-    default:
-        /* Some other error */
-        vshError(ctl, _("virAsprintf failed (errno %d)"), errno);
-    }
-    functionReturn = false;
+    ret = true;

 cleanup:

@@ -1563,7 +1535,7 @@ cleanup:
     vshStorageVolListFree(list);

     /* Return the desired value */
-    return functionReturn;
+    return ret;
 }

 /*
-- 
1.8.5.3

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