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

Eric Blake eblake at redhat.com
Fri Feb 21 22:59:13 UTC 2014


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 at redhat.com>
---
 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




More information about the libvir-list mailing list