[PATCH 4/4] src: Use g_steal_pointer() more

Michal Privoznik posted 4 patches 4 years ago
[PATCH 4/4] src: Use g_steal_pointer() more
Posted by Michal Privoznik 4 years ago
There are few places where the g_steal_pointer() is open coded.
Switch them to calling the g_steal_pointer() function instead.
Generated by the following spatch:

  @ rule1 @
  expression a, b;
  @@
    <...
  - b = a;
    ... when != b
  - a = NULL;
  + b = g_steal_pointer(&a);
    ...>

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/esx/esx_vi.c                      |  6 ++----
 src/hyperv/hyperv_driver.c            |  8 +++-----
 src/hyperv/hyperv_wmi.c               |  3 +--
 src/hypervisor/virhostdev.c           |  3 +--
 src/libxl/xen_common.c                | 12 ++++--------
 src/qemu/qemu_hotplug.c               |  6 ++----
 src/qemu/qemu_migration.c             |  9 +++------
 src/storage/storage_backend_gluster.c |  9 +++------
 src/storage/storage_util.c            |  6 ++----
 src/util/virfile.c                    |  3 +--
 src/util/virlease.c                   |  3 +--
 src/util/virnuma.c                    |  9 +++------
 12 files changed, 26 insertions(+), 51 deletions(-)

diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 80ed6199e3..17401f0a36 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -1033,8 +1033,7 @@ esxVI_Context_LookupManagedObjectsByPath(esxVI_Context *ctx, const char *path)
             if (root != ctx->service->rootFolder)
                 esxVI_ManagedObjectReference_Free(&root);
 
-            root = folder->_reference;
-            folder->_reference = NULL;
+            root = g_steal_pointer(&folder->_reference);
         } else {
             /* Try to lookup item as a datacenter */
             if (esxVI_LookupDatacenter(ctx, item, root, NULL, &ctx->datacenter,
@@ -1087,8 +1086,7 @@ esxVI_Context_LookupManagedObjectsByPath(esxVI_Context *ctx, const char *path)
             if (root != ctx->datacenter->hostFolder)
                 esxVI_ManagedObjectReference_Free(&root);
 
-            root = folder->_reference;
-            folder->_reference = NULL;
+            root = g_steal_pointer(&folder->_reference);
         } else {
             /* Try to lookup item as a compute resource */
             if (esxVI_LookupComputeResource(ctx, item, root, NULL,
diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 7b684e04ba..52851ac507 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -1780,8 +1780,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
 
     priv->version = g_strdup(os->data->Version);
 
-    conn->privateData = priv;
-    priv = NULL;
+    conn->privateData = g_steal_pointer(&priv);
     result = VIR_DRV_OPEN_SUCCESS;
 
  cleanup:
@@ -3518,9 +3517,8 @@ hypervConnectListAllDomains(virConnectPtr conn,
         doms[count++] = domain;
     }
 
-    if (doms)
-        *domains = doms;
-    doms = NULL;
+    if (domains)
+        *domains = g_steal_pointer(&doms);
     ret = count;
 
  cleanup:
diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c
index 6712ef4835..83d28b22aa 100644
--- a/src/hyperv/hyperv_wmi.c
+++ b/src/hyperv/hyperv_wmi.c
@@ -994,8 +994,7 @@ hypervEnumAndPull(hypervPrivate *priv, hypervWqlQuery *wqlQuery,
         response = NULL;
     }
 
-    *list = head;
-    head = NULL;
+    *list = g_steal_pointer(&head);
 
     return 0;
 }
diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
index e44eb7f1d3..bcd5e96c2c 100644
--- a/src/hypervisor/virhostdev.c
+++ b/src/hypervisor/virhostdev.c
@@ -586,8 +586,7 @@ virHostdevRestoreNetConfig(virDomainHostdevDef *hostdev,
          */
         if (MAC) {
             VIR_FREE(adminMAC);
-            adminMAC = MAC;
-            MAC = NULL;
+            adminMAC = g_steal_pointer(&MAC);
         }
 
         ignore_value(virNetDevSetNetConfig(linkdev, vf,
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index fc975f1299..b97ba0a199 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -682,9 +682,8 @@ xenParseVfb(virConf *conf, virDomainDef *def)
             if (xenConfigCopyStringOpt(conf, "keymap", &graphics->data.vnc.keymap) < 0)
                 goto cleanup;
             def->graphics = g_new0(virDomainGraphicsDef *, 1);
-            def->graphics[0] = graphics;
+            def->graphics[0] = g_steal_pointer(&graphics);
             def->ngraphics = 1;
-            graphics = NULL;
         } else {
             if (xenConfigGetBool(conf, "sdl", &val, 0) < 0)
                 goto cleanup;
@@ -696,9 +695,8 @@ xenParseVfb(virConf *conf, virDomainDef *def)
                 if (xenConfigCopyStringOpt(conf, "xauthority", &graphics->data.sdl.xauth) < 0)
                     goto cleanup;
                 def->graphics = g_new0(virDomainGraphicsDef *, 1);
-                def->graphics[0] = graphics;
+                def->graphics[0] = g_steal_pointer(&graphics);
                 def->ngraphics = 1;
-                graphics = NULL;
             }
         }
     }
@@ -774,9 +772,8 @@ xenParseVfb(virConf *conf, virDomainDef *def)
                 VIR_FREE(listenAddr);
             }
             def->graphics = g_new0(virDomainGraphicsDef *, 1);
-            def->graphics[0] = graphics;
+            def->graphics[0] = g_steal_pointer(&graphics);
             def->ngraphics = 1;
-            graphics = NULL;
         } else {
             if (xenHandleConfGetValueStringListErrors(rc) < 0)
                 goto cleanup;
@@ -953,9 +950,8 @@ xenParseCharDev(virConf *conf, virDomainDef *def, const char *nativeFormat)
 
             chr->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL;
             chr->target.port = 0;
-            def->parallels[0] = chr;
+            def->parallels[0] = g_steal_pointer(&chr);
             def->nparallels++;
-            chr = NULL;
         }
 
         /* Try to get the list of values to support multiple serial ports */
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index d801559057..c9548d7f35 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4297,8 +4297,7 @@ qemuDomainChangeGraphics(virQEMUDriver *driver,
 
             /* Steal the new dev's  char * reference */
             VIR_FREE(olddev->data.vnc.auth.passwd);
-            olddev->data.vnc.auth.passwd = dev->data.vnc.auth.passwd;
-            dev->data.vnc.auth.passwd = NULL;
+            olddev->data.vnc.auth.passwd = g_steal_pointer(&dev->data.vnc.auth.passwd);
             olddev->data.vnc.auth.validTo = dev->data.vnc.auth.validTo;
             olddev->data.vnc.auth.expires = dev->data.vnc.auth.expires;
             olddev->data.vnc.auth.connected = dev->data.vnc.auth.connected;
@@ -4345,8 +4344,7 @@ qemuDomainChangeGraphics(virQEMUDriver *driver,
 
             /* Steal the new dev's char * reference */
             VIR_FREE(olddev->data.spice.auth.passwd);
-            olddev->data.spice.auth.passwd = dev->data.spice.auth.passwd;
-            dev->data.spice.auth.passwd = NULL;
+            olddev->data.spice.auth.passwd = g_steal_pointer(&dev->data.spice.auth.passwd);
             olddev->data.spice.auth.validTo = dev->data.spice.auth.validTo;
             olddev->data.spice.auth.expires = dev->data.spice.auth.expires;
             olddev->data.spice.auth.connected = dev->data.spice.auth.connected;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 2635ef1162..09b8c415f4 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3029,8 +3029,7 @@ qemuMigrationDstPrepareAny(virQEMUDriver *driver,
     if (mig->lockState) {
         VIR_DEBUG("Received lockstate %s", mig->lockState);
         VIR_FREE(priv->lockState);
-        priv->lockState = mig->lockState;
-        mig->lockState = NULL;
+        priv->lockState = g_steal_pointer(&mig->lockState);
     } else {
         VIR_DEBUG("Received no lockstate");
     }
@@ -5598,8 +5597,7 @@ qemuMigrationDstPersist(virQEMUDriver *driver,
  error:
     virDomainDefFree(vm->newDef);
     vm->persistent = oldPersist;
-    vm->newDef = oldDef;
-    oldDef = NULL;
+    vm->newDef = g_steal_pointer(&oldDef);
     return -1;
 }
 
@@ -5778,8 +5776,7 @@ qemuMigrationDstFinish(virQEMUDriver *driver,
     }
 
     if (mig->jobInfo) {
-        jobInfo = mig->jobInfo;
-        mig->jobInfo = NULL;
+        jobInfo = g_steal_pointer(&mig->jobInfo);
 
         if (jobInfo->sent && timeReceived) {
             jobInfo->timeDelta = timeReceived - jobInfo->sent;
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
index aacc23663e..52007db2ba 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -290,13 +290,10 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterState *state,
     if (meta->capacity)
         vol->target.capacity = meta->capacity;
     if (meta->encryption) {
-        vol->target.encryption = meta->encryption;
-        meta->encryption = NULL;
+        vol->target.encryption = g_steal_pointer(&meta->encryption);
     }
-    vol->target.features = meta->features;
-    meta->features = NULL;
-    vol->target.compat = meta->compat;
-    meta->compat = NULL;
+    vol->target.features = g_steal_pointer(&meta->features);
+    vol->target.compat = g_steal_pointer(&meta->compat);
 
     *volptr = g_steal_pointer(&vol);
     ret = 0;
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index a680f14afc..f88d225e96 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -3443,8 +3443,7 @@ storageBackendProbeTarget(virStorageSource *target,
 
             target->backingStore = virStorageSourceNew();
             target->backingStore->type = VIR_STORAGE_TYPE_NETWORK;
-            target->backingStore->path = meta->backingStoreRaw;
-            meta->backingStoreRaw = NULL;
+            target->backingStore->path = g_steal_pointer(&meta->backingStoreRaw);
             target->backingStore->format = VIR_STORAGE_FILE_RAW;
         }
 
@@ -3479,8 +3478,7 @@ storageBackendProbeTarget(virStorageSource *target,
         if (meta->encryption->payload_offset != -1)
             target->capacity -= meta->encryption->payload_offset * 512;
 
-        *encryption = meta->encryption;
-        meta->encryption = NULL;
+        *encryption = g_steal_pointer(&meta->encryption);
 
         /* XXX ideally we'd fill in secret UUID here
          * but we cannot guarantee 'conn' is non-NULL
diff --git a/src/util/virfile.c b/src/util/virfile.c
index d6faf7e3d2..2d9c89ebbe 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -967,8 +967,7 @@ int virFileNBDDeviceAssociate(const char *file,
 
     VIR_DEBUG("Associated NBD device %s with file %s and format %s",
               nbddev, file, fmtstr);
-    *dev = nbddev;
-    nbddev = NULL;
+    *dev = g_steal_pointer(&nbddev);
 
     return 0;
 }
diff --git a/src/util/virlease.c b/src/util/virlease.c
index 0e28a47dd2..48b0f6fe57 100644
--- a/src/util/virlease.c
+++ b/src/util/virlease.c
@@ -254,7 +254,6 @@ virLeaseNew(virJSONValue **lease_ret,
     if (virJSONValueObjectAppendNumberLong(lease_new, "expiry-time", expirytime) < 0)
         return -1;
 
-    *lease_ret = lease_new;
-    lease_new = NULL;
+    *lease_ret = g_steal_pointer(&lease_new);
     return 0;
 }
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 7c892d6267..2306ab0cb1 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -839,16 +839,13 @@ virNumaGetPages(int node,
     } while (exchange);
 
     if (pages_size) {
-        *pages_size = tmp_size;
-        tmp_size = NULL;
+        *pages_size = g_steal_pointer(&tmp_size);
     }
     if (pages_avail) {
-        *pages_avail = tmp_avail;
-        tmp_avail = NULL;
+        *pages_avail = g_steal_pointer(&tmp_avail);
     }
     if (pages_free) {
-        *pages_free = tmp_free;
-        tmp_free = NULL;
+        *pages_free = g_steal_pointer(&tmp_free);
     }
     *npages = ntmp;
     return 0;
-- 
2.34.1

Re: [PATCH 4/4] src: Use g_steal_pointer() more
Posted by Erik Skultety 4 years ago
On Mon, Jan 31, 2022 at 03:53:42PM +0100, Michal Privoznik wrote:
> There are few places where the g_steal_pointer() is open coded.
> Switch them to calling the g_steal_pointer() function instead.
> Generated by the following spatch:
> 
>   @ rule1 @
>   expression a, b;
>   @@
>     <...
>   - b = a;
>     ... when != b
>   - a = NULL;
>   + b = g_steal_pointer(&a);
>     ...>
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
...

> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 7b684e04ba..52851ac507 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -1780,8 +1780,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
>  
>      priv->version = g_strdup(os->data->Version);
>  
> -    conn->privateData = priv;
> -    priv = NULL;
> +    conn->privateData = g_steal_pointer(&priv);
>      result = VIR_DRV_OPEN_SUCCESS;
>  
>   cleanup:
> @@ -3518,9 +3517,8 @@ hypervConnectListAllDomains(virConnectPtr conn,
>          doms[count++] = domain;
>      }
>  
> -    if (doms)
> -        *domains = doms;
> -    doms = NULL;
> +    if (domains)
> +        *domains = g_steal_pointer(&doms);

^this is not semantically identical, you need to fix it manually before pushing

Reviewed-by: Erik Skultety <eskultet@redhat.com>

Re: [PATCH 4/4] src: Use g_steal_pointer() more
Posted by Michal Prívozník 4 years ago
On 2/1/22 17:18, Erik Skultety wrote:
> On Mon, Jan 31, 2022 at 03:53:42PM +0100, Michal Privoznik wrote:
>> There are few places where the g_steal_pointer() is open coded.
>> Switch them to calling the g_steal_pointer() function instead.
>> Generated by the following spatch:
>>
>>   @ rule1 @
>>   expression a, b;
>>   @@
>>     <...
>>   - b = a;
>>     ... when != b
>>   - a = NULL;
>>   + b = g_steal_pointer(&a);
>>     ...>
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
> ...
> 
>> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
>> index 7b684e04ba..52851ac507 100644
>> --- a/src/hyperv/hyperv_driver.c
>> +++ b/src/hyperv/hyperv_driver.c
>> @@ -1780,8 +1780,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
>>  
>>      priv->version = g_strdup(os->data->Version);
>>  
>> -    conn->privateData = priv;
>> -    priv = NULL;
>> +    conn->privateData = g_steal_pointer(&priv);
>>      result = VIR_DRV_OPEN_SUCCESS;
>>  
>>   cleanup:
>> @@ -3518,9 +3517,8 @@ hypervConnectListAllDomains(virConnectPtr conn,
>>          doms[count++] = domain;
>>      }
>>  
>> -    if (doms)
>> -        *domains = doms;
>> -    doms = NULL;
>> +    if (domains)
>> +        *domains = g_steal_pointer(&doms);
> 
> ^this is not semantically identical, you need to fix it manually before pushing
> 
> Reviewed-by: Erik Skultety <eskultet@redhat.com>
> 

Yeah, hyperv code doesn't get as much love as other drivers, but
basically, @domains is allocated iff @doms != NULL, there's the
following code earlier in the function:

    if (domains) {
        doms = g_new0(virDomainPtr, 1);
        ndoms = 1;
    }

I find the new version easier to read, since @domains is an argument of
the function. Do you want me to post a separate patch that does
s/doms/domains/?

Michal

Re: [PATCH 4/4] src: Use g_steal_pointer() more
Posted by Erik Skultety 4 years ago
On Tue, Feb 01, 2022 at 05:23:52PM +0100, Michal Prívozník wrote:
> On 2/1/22 17:18, Erik Skultety wrote:
> > On Mon, Jan 31, 2022 at 03:53:42PM +0100, Michal Privoznik wrote:
> >> There are few places where the g_steal_pointer() is open coded.
> >> Switch them to calling the g_steal_pointer() function instead.
> >> Generated by the following spatch:
> >>
> >>   @ rule1 @
> >>   expression a, b;
> >>   @@
> >>     <...
> >>   - b = a;
> >>     ... when != b
> >>   - a = NULL;
> >>   + b = g_steal_pointer(&a);
> >>     ...>
> >>
> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> >> ---
> > ...
> > 
> >> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> >> index 7b684e04ba..52851ac507 100644
> >> --- a/src/hyperv/hyperv_driver.c
> >> +++ b/src/hyperv/hyperv_driver.c
> >> @@ -1780,8 +1780,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
> >>  
> >>      priv->version = g_strdup(os->data->Version);
> >>  
> >> -    conn->privateData = priv;
> >> -    priv = NULL;
> >> +    conn->privateData = g_steal_pointer(&priv);
> >>      result = VIR_DRV_OPEN_SUCCESS;
> >>  
> >>   cleanup:
> >> @@ -3518,9 +3517,8 @@ hypervConnectListAllDomains(virConnectPtr conn,
> >>          doms[count++] = domain;
> >>      }
> >>  
> >> -    if (doms)
> >> -        *domains = doms;
> >> -    doms = NULL;
> >> +    if (domains)
> >> +        *domains = g_steal_pointer(&doms);
> > 
> > ^this is not semantically identical, you need to fix it manually before pushing
> > 
> > Reviewed-by: Erik Skultety <eskultet@redhat.com>
> > 
> 
> Yeah, hyperv code doesn't get as much love as other drivers, but
> basically, @domains is allocated iff @doms != NULL, there's the
> following code earlier in the function:
> 
>     if (domains) {
>         doms = g_new0(virDomainPtr, 1);
>         ndoms = 1;
>     }
> 
> I find the new version easier to read, since @domains is an argument of
> the function. Do you want me to post a separate patch that does

Well, it's not consistent with the rest which is not good for commit history
in case this introduced a bug for some reason...

> s/doms/domains/?

It would probably need to a tiny bit more than that, e.g.:

diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
index 7b684e04ba..af71eef7e2 100644
--- a/src/hyperv/hyperv_driver.c
+++ b/src/hyperv/hyperv_driver.c
@@ -3423,7 +3423,6 @@ hypervConnectListAllDomains(virConnectPtr conn,
     g_autoptr(Msvm_ComputerSystem) computerSystemList = NULL;
     Msvm_ComputerSystem *computerSystem = NULL;
     size_t ndoms;
-    virDomainPtr domain;
     virDomainPtr *doms = NULL;
     int count = 0;
     int ret = -1;
@@ -3475,6 +3474,7 @@ hypervConnectListAllDomains(virConnectPtr conn,

     for (computerSystem = computerSystemList; computerSystem != NULL;
          computerSystem = computerSystem->next) {
+        virDomainPtr domain = NULL;

         /* filter by domain state */
         if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE)) {
@@ -3509,13 +3509,11 @@ hypervConnectListAllDomains(virConnectPtr conn,

         VIR_RESIZE_N(doms, ndoms, count, 2);

-        domain = NULL;
-
         if (hypervMsvmComputerSystemToDomain(conn, computerSystem,
                                              &domain) < 0)
             goto cleanup;

-        doms[count++] = domain;
+        doms[count++] = g_steal_pointer(domain);
     }

     if (doms)


Erik

Re: [PATCH 4/4] src: Use g_steal_pointer() more
Posted by Michal Prívozník 4 years ago
On 2/1/22 17:59, Erik Skultety wrote:
> On Tue, Feb 01, 2022 at 05:23:52PM +0100, Michal Prívozník wrote:
>> On 2/1/22 17:18, Erik Skultety wrote:
>>> On Mon, Jan 31, 2022 at 03:53:42PM +0100, Michal Privoznik wrote:
>>>> There are few places where the g_steal_pointer() is open coded.
>>>> Switch them to calling the g_steal_pointer() function instead.
>>>> Generated by the following spatch:
>>>>
>>>>   @ rule1 @
>>>>   expression a, b;
>>>>   @@
>>>>     <...
>>>>   - b = a;
>>>>     ... when != b
>>>>   - a = NULL;
>>>>   + b = g_steal_pointer(&a);
>>>>     ...>
>>>>
>>>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>>>> ---
>>> ...
>>>
>>>> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
>>>> index 7b684e04ba..52851ac507 100644
>>>> --- a/src/hyperv/hyperv_driver.c
>>>> +++ b/src/hyperv/hyperv_driver.c
>>>> @@ -1780,8 +1780,7 @@ hypervConnectOpen(virConnectPtr conn, virConnectAuthPtr auth,
>>>>  
>>>>      priv->version = g_strdup(os->data->Version);
>>>>  
>>>> -    conn->privateData = priv;
>>>> -    priv = NULL;
>>>> +    conn->privateData = g_steal_pointer(&priv);
>>>>      result = VIR_DRV_OPEN_SUCCESS;
>>>>  
>>>>   cleanup:
>>>> @@ -3518,9 +3517,8 @@ hypervConnectListAllDomains(virConnectPtr conn,
>>>>          doms[count++] = domain;
>>>>      }
>>>>  
>>>> -    if (doms)
>>>> -        *domains = doms;
>>>> -    doms = NULL;
>>>> +    if (domains)
>>>> +        *domains = g_steal_pointer(&doms);
>>>
>>> ^this is not semantically identical, you need to fix it manually before pushing
>>>
>>> Reviewed-by: Erik Skultety <eskultet@redhat.com>
>>>
>>
>> Yeah, hyperv code doesn't get as much love as other drivers, but
>> basically, @domains is allocated iff @doms != NULL, there's the
>> following code earlier in the function:
>>
>>     if (domains) {
>>         doms = g_new0(virDomainPtr, 1);
>>         ndoms = 1;
>>     }
>>
>> I find the new version easier to read, since @domains is an argument of
>> the function. Do you want me to post a separate patch that does
> 
> Well, it's not consistent with the rest which is not good for commit history
> in case this introduced a bug for some reason...

It's not, but I think it's trivial enough to be contained in this
commit. But alright, let me keep the original condition and push.

> 
>> s/doms/domains/?
> 
> It would probably need to a tiny bit more than that, e.g.:
> 
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 7b684e04ba..af71eef7e2 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -3423,7 +3423,6 @@ hypervConnectListAllDomains(virConnectPtr conn,
>      g_autoptr(Msvm_ComputerSystem) computerSystemList = NULL;
>      Msvm_ComputerSystem *computerSystem = NULL;
>      size_t ndoms;
> -    virDomainPtr domain;
>      virDomainPtr *doms = NULL;
>      int count = 0;
>      int ret = -1;
> @@ -3475,6 +3474,7 @@ hypervConnectListAllDomains(virConnectPtr conn,
> 
>      for (computerSystem = computerSystemList; computerSystem != NULL;
>           computerSystem = computerSystem->next) {
> +        virDomainPtr domain = NULL;
> 
>          /* filter by domain state */
>          if (MATCH(VIR_CONNECT_LIST_DOMAINS_FILTERS_STATE)) {
> @@ -3509,13 +3509,11 @@ hypervConnectListAllDomains(virConnectPtr conn,
> 
>          VIR_RESIZE_N(doms, ndoms, count, 2);
> 
> -        domain = NULL;
> -
>          if (hypervMsvmComputerSystemToDomain(conn, computerSystem,
>                                               &domain) < 0)
>              goto cleanup;
> 
> -        doms[count++] = domain;
> +        doms[count++] = g_steal_pointer(domain);
>      }
> 
>      if (doms)

This is unrelated. Worth pushing, but unrelated. Note, there are three
variables: doms, domain, and domains. My patch touches only the first
and the last one.

Michal