[PATCH] vsh: Don't check for OOM in vshGetTypedParamValue()

Michal Privoznik 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/68835bb2922f3255cad089265e096e39a72cdd5e.1632478365.git.mprivozn@redhat.com
tools/vsh.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)
[PATCH] vsh: Don't check for OOM in vshGetTypedParamValue()
Posted by Michal Privoznik 2 years, 7 months ago
Both function description and function itself mention check for
OOM which can't happen really. There was a bug in glib where
g_strdup_*() might have not aborted on OOM, but we have our own
implementation when dealing with broken glib (see
vir_g_strdup_printf()). Therefore, checking for OOM is redundant
and can never be true.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 tools/vsh.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index e81369f224..189c452f73 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -1806,51 +1806,44 @@ vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, int *timeout)
  * ---------------
  */
 
-/* Return a non-NULL string representation of a typed parameter; exit
- * if we are out of memory.  */
+/* Return a non-NULL string representation of a typed parameter; exit on
+ * unknown type. */
 char *
 vshGetTypedParamValue(vshControl *ctl, virTypedParameterPtr item)
 {
-    char *str = NULL;
-
     switch (item->type) {
     case VIR_TYPED_PARAM_INT:
-        str = g_strdup_printf("%d", item->value.i);
+        return g_strdup_printf("%d", item->value.i);
         break;
 
     case VIR_TYPED_PARAM_UINT:
-        str = g_strdup_printf("%u", item->value.ui);
+        return g_strdup_printf("%u", item->value.ui);
         break;
 
     case VIR_TYPED_PARAM_LLONG:
-        str = g_strdup_printf("%lld", item->value.l);
+        return g_strdup_printf("%lld", item->value.l);
         break;
 
     case VIR_TYPED_PARAM_ULLONG:
-        str = g_strdup_printf("%llu", item->value.ul);
+        return g_strdup_printf("%llu", item->value.ul);
         break;
 
     case VIR_TYPED_PARAM_DOUBLE:
-        str = g_strdup_printf("%f", item->value.d);
+        return g_strdup_printf("%f", item->value.d);
         break;
 
     case VIR_TYPED_PARAM_BOOLEAN:
-        str = g_strdup(item->value.b ? _("yes") : _("no"));
+        return g_strdup(item->value.b ? _("yes") : _("no"));
         break;
 
     case VIR_TYPED_PARAM_STRING:
-        str = g_strdup(item->value.s);
+        return g_strdup(item->value.s);
         break;
 
     default:
         vshError(ctl, _("unimplemented parameter type %d"), item->type);
-    }
-
-    if (!str) {
-        vshError(ctl, "%s", _("Out of memory"));
         exit(EXIT_FAILURE);
     }
-    return str;
 }
 
 void
-- 
2.32.0

Re: [PATCH] vsh: Don't check for OOM in vshGetTypedParamValue()
Posted by Ján Tomko 2 years, 7 months ago
On a Friday in 2021, Michal Privoznik wrote:
>Both function description and function itself mention check for
>OOM which can't happen really. There was a bug in glib where
>g_strdup_*() might have not aborted on OOM, but we have our own
>implementation when dealing with broken glib (see
>vir_g_strdup_printf()). Therefore, checking for OOM is redundant
>and can never be true.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> tools/vsh.c | 25 +++++++++----------------
> 1 file changed, 9 insertions(+), 16 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano