[PATCH 5/7] qemuMigrationCapsToJSON: Refactor variable cleanup

Peter Krempa posted 7 patches 5 years, 5 months ago
[PATCH 5/7] qemuMigrationCapsToJSON: Refactor variable cleanup
Posted by Peter Krempa 5 years, 5 months ago
Use automatic memory allocation and move variables into correct scope to
simplify the code and remove the need for a 'error:' label.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_migration_params.c | 24 +++++++-----------------
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index c22362735a..9b6601367a 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -759,14 +759,12 @@ virJSONValuePtr
 qemuMigrationCapsToJSON(virBitmapPtr caps,
                         virBitmapPtr states)
 {
-    virJSONValuePtr json = NULL;
-    virJSONValuePtr cap = NULL;
+    g_autoptr(virJSONValue) json = virJSONValueNewArray();
     qemuMigrationCapability bit;
-    const char *name;
-
-    json = virJSONValueNewArray();

     for (bit = 0; bit < QEMU_MIGRATION_CAP_LAST; bit++) {
+        g_autoptr(virJSONValue) cap = virJSONValueNewObject();
+        const char *name = qemuMigrationCapabilityTypeToString(bit);
         bool supported = false;
         bool state = false;

@@ -776,27 +774,19 @@ qemuMigrationCapsToJSON(virBitmapPtr caps,

         ignore_value(virBitmapGetBit(states, bit, &state));

-        cap = virJSONValueNewObject();
-
-        name = qemuMigrationCapabilityTypeToString(bit);
         if (virJSONValueObjectAppendString(cap, "capability", name) < 0)
-            goto error;
+            return NULL;

         if (virJSONValueObjectAppendBoolean(cap, "state", state) < 0)
-            goto error;
+            return NULL;

         if (virJSONValueArrayAppend(json, cap) < 0)
-            goto error;
+            return NULL;

         cap = NULL;
     }

-    return json;
-
- error:
-    virJSONValueFree(json);
-    virJSONValueFree(cap);
-    return NULL;
+    return g_steal_pointer(&json);
 }


-- 
2.26.2

Re: [PATCH 5/7] qemuMigrationCapsToJSON: Refactor variable cleanup
Posted by Ján Tomko 5 years, 5 months ago
On a Monday in 2020, Peter Krempa wrote:
>Use automatic memory allocation and move variables into correct scope to
>simplify the code and remove the need for a 'error:' label.
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/qemu/qemu_migration_params.c | 24 +++++++-----------------
> 1 file changed, 7 insertions(+), 17 deletions(-)
>
>diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
>index c22362735a..9b6601367a 100644
>--- a/src/qemu/qemu_migration_params.c
>+++ b/src/qemu/qemu_migration_params.c
>@@ -759,14 +759,12 @@ virJSONValuePtr
> qemuMigrationCapsToJSON(virBitmapPtr caps,
>                         virBitmapPtr states)
> {
>-    virJSONValuePtr json = NULL;
>-    virJSONValuePtr cap = NULL;
>+    g_autoptr(virJSONValue) json = virJSONValueNewArray();
>     qemuMigrationCapability bit;
>-    const char *name;
>-
>-    json = virJSONValueNewArray();
>
>     for (bit = 0; bit < QEMU_MIGRATION_CAP_LAST; bit++) {
>+        g_autoptr(virJSONValue) cap = virJSONValueNewObject();
>+        const char *name = qemuMigrationCapabilityTypeToString(bit);

I don't like allocating the JSON object and converting the string
if we might 'continue' without using it, but I doubt it will have a
measurable effect.

>         bool supported = false;
>         bool state = false;
>

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

Jano
Re: [PATCH 5/7] qemuMigrationCapsToJSON: Refactor variable cleanup
Posted by Peter Krempa 5 years, 5 months ago
On Mon, Aug 24, 2020 at 15:27:40 +0200, Ján Tomko wrote:
> On a Monday in 2020, Peter Krempa wrote:
> > Use automatic memory allocation and move variables into correct scope to
> > simplify the code and remove the need for a 'error:' label.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> > src/qemu/qemu_migration_params.c | 24 +++++++-----------------
> > 1 file changed, 7 insertions(+), 17 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
> > index c22362735a..9b6601367a 100644
> > --- a/src/qemu/qemu_migration_params.c
> > +++ b/src/qemu/qemu_migration_params.c
> > @@ -759,14 +759,12 @@ virJSONValuePtr
> > qemuMigrationCapsToJSON(virBitmapPtr caps,
> >                         virBitmapPtr states)
> > {
> > -    virJSONValuePtr json = NULL;
> > -    virJSONValuePtr cap = NULL;
> > +    g_autoptr(virJSONValue) json = virJSONValueNewArray();
> >     qemuMigrationCapability bit;
> > -    const char *name;
> > -
> > -    json = virJSONValueNewArray();
> > 
> >     for (bit = 0; bit < QEMU_MIGRATION_CAP_LAST; bit++) {
> > +        g_autoptr(virJSONValue) cap = virJSONValueNewObject();
> > +        const char *name = qemuMigrationCapabilityTypeToString(bit);
> 
> I don't like allocating the JSON object and converting the string
> if we might 'continue' without using it, but I doubt it will have a
> measurable effect.

I have an idea how to fix it, but it will not go well under this patch.
I'll follow up with another one changing the code a bit.