[libvirt] [PATCH 2/4] conf: Use VIR_AUTOPTR(virBitmap) in domain_conf

John Ferlan posted 4 patches 6 years, 11 months ago
There is a newer version of this series
[libvirt] [PATCH 2/4] conf: Use VIR_AUTOPTR(virBitmap) in domain_conf
Posted by John Ferlan 6 years, 11 months ago
Let's make use of the auto __cleanup capabilities cleaning up any
now unnecessary goto paths.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/conf/domain_conf.c | 53 ++++++++++++++++--------------------------
 1 file changed, 20 insertions(+), 33 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ceeb247ef4..ddcb76f05d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1803,8 +1803,8 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
                                  virBitmapPtr autoCpuset)
 {
     int maxvcpus = virDomainDefGetVcpusMax(def);
-    virBitmapPtr allcpumap = NULL;
     size_t i;
+    VIR_AUTOPTR(virBitmap) allcpumap = NULL;
 
     if (hostcpus < 0)
         return -1;
@@ -1831,7 +1831,6 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
         virBitmapToDataBuf(bitmap, VIR_GET_CPUMAP(cpumaps, maplen, i), maplen);
     }
 
-    virBitmapFree(allcpumap);
     return i;
 }
 
@@ -2984,11 +2983,10 @@ static int
 virDomainIOThreadIDDefArrayInit(virDomainDefPtr def,
                                 unsigned int iothreads)
 {
-    int retval = -1;
     size_t i;
     ssize_t nxt = -1;
     virDomainIOThreadIDDefPtr iothrid = NULL;
-    virBitmapPtr thrmap = NULL;
+    VIR_AUTOPTR(virBitmap) thrmap = NULL;
 
     /* Same value (either 0 or some number), then we have none to fill in or
      * the iothreadid array was filled from the XML
@@ -2998,7 +2996,7 @@ virDomainIOThreadIDDefArrayInit(virDomainDefPtr def,
 
     /* iothread's are numbered starting at 1, account for that */
     if (!(thrmap = virBitmapNew(iothreads + 1)))
-        goto error;
+        return -1;
     virBitmapSetAll(thrmap);
 
     /* Clear 0 since we don't use it, then mark those which are
@@ -3010,27 +3008,23 @@ virDomainIOThreadIDDefArrayInit(virDomainDefPtr def,
 
     /* resize array */
     if (VIR_REALLOC_N(def->iothreadids, iothreads) < 0)
-        goto error;
+        return -1;
 
     /* Populate iothreadids[] using the set bit number from thrmap */
     while (def->niothreadids < iothreads) {
         if ((nxt = virBitmapNextSetBit(thrmap, nxt)) < 0) {
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                            _("failed to populate iothreadids"));
-            goto error;
+            return -1;
         }
         if (VIR_ALLOC(iothrid) < 0)
-            goto error;
+            return -1;
         iothrid->iothread_id = nxt;
         iothrid->autofill = true;
         def->iothreadids[def->niothreadids++] = iothrid;
     }
 
-    retval = 0;
-
- error:
-    virBitmapFree(thrmap);
-    return retval;
+    return 0;
 }
 
 
@@ -18327,9 +18321,9 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node,
 {
     int ret = -1;
     virDomainIOThreadIDDefPtr iothrid;
-    virBitmapPtr cpumask = NULL;
     unsigned int iothreadid;
     char *tmp = NULL;
+    VIR_AUTOPTR(virBitmap) cpumask = NULL;
 
     if (!(tmp = virXMLPropString(node, "iothread"))) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -18385,7 +18379,6 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node,
 
  cleanup:
     VIR_FREE(tmp);
-    virBitmapFree(cpumask);
     return ret;
 }
 
@@ -18397,8 +18390,9 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node,
 static virBitmapPtr
 virDomainEmulatorPinDefParseXML(xmlNodePtr node)
 {
-    virBitmapPtr def = NULL;
+    virBitmapPtr ret = NULL;
     char *tmp = NULL;
+    VIR_AUTOPTR(virBitmap) def = NULL;
 
     if (!(tmp = virXMLPropString(node, "cpuset"))) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -18412,14 +18406,14 @@ virDomainEmulatorPinDefParseXML(xmlNodePtr node)
     if (virBitmapIsAllClear(def)) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("Invalid value of 'cpuset': %s"), tmp);
-        virBitmapFree(def);
-        def = NULL;
         goto cleanup;
     }
 
+    VIR_STEAL_PTR(ret, def);
+
  cleanup:
     VIR_FREE(tmp);
-    return def;
+    return ret;
 }
 
 
@@ -18778,35 +18772,30 @@ virDomainThreadSchedParseHelper(xmlNodePtr node,
                                 virDomainDefPtr def)
 {
     ssize_t next = -1;
-    virBitmapPtr map = NULL;
     virDomainThreadSchedParamPtr sched;
     virProcessSchedPolicy policy;
     int priority;
-    int ret = -1;
+    VIR_AUTOPTR(virBitmap) map = NULL;
 
     if (!(map = virDomainSchedulerParse(node, name, &policy, &priority)))
-        goto cleanup;
+        return -1;
 
     while ((next = virBitmapNextSetBit(map, next)) > -1) {
         if (!(sched = func(def, next)))
-            goto cleanup;
+            return -1;
 
         if (sched->policy != VIR_PROC_POLICY_NONE) {
             virReportError(VIR_ERR_XML_DETAIL,
                            _("%ssched attributes 'vcpus' must not overlap"),
                            name);
-            goto cleanup;
+            return -1;
         }
 
         sched->policy = policy;
         sched->priority = priority;
     }
 
-    ret = 0;
-
- cleanup:
-    virBitmapFree(map);
-    return ret;
+    return 0;
 }
 
 
@@ -19509,12 +19498,12 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
 {
     xmlNodePtr oldnode = ctxt->node;
     xmlNodePtr *nodes = NULL;
-    virBitmapPtr vcpus = NULL;
     virResctrlAllocPtr alloc = NULL;
     virDomainResctrlDefPtr resctrl = NULL;
     ssize_t i = 0;
     int n;
     int ret = -1;
+    VIR_AUTOPTR(virBitmap) vcpus = NULL;
 
     ctxt->node = node;
 
@@ -19574,7 +19563,6 @@ virDomainCachetuneDefParse(virDomainDefPtr def,
     ctxt->node = oldnode;
     virDomainResctrlDefFree(resctrl);
     virObjectUnref(alloc);
-    virBitmapFree(vcpus);
     VIR_FREE(nodes);
     return ret;
 }
@@ -19728,9 +19716,9 @@ virDomainMemorytuneDefParse(virDomainDefPtr def,
 {
     xmlNodePtr oldnode = ctxt->node;
     xmlNodePtr *nodes = NULL;
-    virBitmapPtr vcpus = NULL;
     virResctrlAllocPtr alloc = NULL;
     virDomainResctrlDefPtr resctrl = NULL;
+    VIR_AUTOPTR(virBitmap) vcpus = NULL;
 
     ssize_t i = 0;
     int n;
@@ -19789,7 +19777,6 @@ virDomainMemorytuneDefParse(virDomainDefPtr def,
     ctxt->node = oldnode;
     virDomainResctrlDefFree(resctrl);
     virObjectUnref(alloc);
-    virBitmapFree(vcpus);
     VIR_FREE(nodes);
     return ret;
 }
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] conf: Use VIR_AUTOPTR(virBitmap) in domain_conf
Posted by Ján Tomko 6 years, 11 months ago
On Wed, Feb 20, 2019 at 09:46:57AM -0500, John Ferlan wrote:
>Let's make use of the auto __cleanup capabilities cleaning up any
>now unnecessary goto paths.

My clang 7 and gcc 8.2 both happily compile code where the cleanup:
label only contains a return statement, the goto paths can be cleaned
up later.

>
>Signed-off-by: John Ferlan <jferlan@redhat.com>
>---
> src/conf/domain_conf.c | 53 ++++++++++++++++--------------------------
> 1 file changed, 20 insertions(+), 33 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index ceeb247ef4..ddcb76f05d 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -1803,8 +1803,8 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
>                                  virBitmapPtr autoCpuset)
> {
>     int maxvcpus = virDomainDefGetVcpusMax(def);
>-    virBitmapPtr allcpumap = NULL;
>     size_t i;
>+    VIR_AUTOPTR(virBitmap) allcpumap = NULL;
>
>     if (hostcpus < 0)
>         return -1;
>@@ -1831,7 +1831,6 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
>         virBitmapToDataBuf(bitmap, VIR_GET_CPUMAP(cpumaps, maplen, i), maplen);
>     }
>
>-    virBitmapFree(allcpumap);
>     return i;
> }
>
>@@ -2984,11 +2983,10 @@ static int
> virDomainIOThreadIDDefArrayInit(virDomainDefPtr def,
>                                 unsigned int iothreads)
> {
>-    int retval = -1;
>     size_t i;
>     ssize_t nxt = -1;
>     virDomainIOThreadIDDefPtr iothrid = NULL;
>-    virBitmapPtr thrmap = NULL;
>+    VIR_AUTOPTR(virBitmap) thrmap = NULL;
>
>     /* Same value (either 0 or some number), then we have none to fill in or
>      * the iothreadid array was filled from the XML
>@@ -2998,7 +2996,7 @@ virDomainIOThreadIDDefArrayInit(virDomainDefPtr def,
>
>     /* iothread's are numbered starting at 1, account for that */
>     if (!(thrmap = virBitmapNew(iothreads + 1)))
>-        goto error;
>+        return -1;
>     virBitmapSetAll(thrmap);
>
>     /* Clear 0 since we don't use it, then mark those which are
>@@ -3010,27 +3008,23 @@ virDomainIOThreadIDDefArrayInit(virDomainDefPtr def,
>
>     /* resize array */
>     if (VIR_REALLOC_N(def->iothreadids, iothreads) < 0)
>-        goto error;
>+        return -1;
>
>     /* Populate iothreadids[] using the set bit number from thrmap */
>     while (def->niothreadids < iothreads) {
>         if ((nxt = virBitmapNextSetBit(thrmap, nxt)) < 0) {
>             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                            _("failed to populate iothreadids"));
>-            goto error;
>+            return -1;
>         }
>         if (VIR_ALLOC(iothrid) < 0)
>-            goto error;
>+            return -1;
>         iothrid->iothread_id = nxt;
>         iothrid->autofill = true;
>         def->iothreadids[def->niothreadids++] = iothrid;
>     }
>
>-    retval = 0;
>-
>- error:
>-    virBitmapFree(thrmap);
>-    return retval;
>+    return 0;
> }
>
>
>@@ -18327,9 +18321,9 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node,
> {
>     int ret = -1;
>     virDomainIOThreadIDDefPtr iothrid;
>-    virBitmapPtr cpumask = NULL;
>     unsigned int iothreadid;
>     char *tmp = NULL;
>+    VIR_AUTOPTR(virBitmap) cpumask = NULL;
>
>     if (!(tmp = virXMLPropString(node, "iothread"))) {
>         virReportError(VIR_ERR_XML_ERROR, "%s",
>@@ -18385,7 +18379,6 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node,
>
>  cleanup:
>     VIR_FREE(tmp);
>-    virBitmapFree(cpumask);
>     return ret;
> }
>
>@@ -18397,8 +18390,9 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node,
> static virBitmapPtr
> virDomainEmulatorPinDefParseXML(xmlNodePtr node)
> {
>-    virBitmapPtr def = NULL;
>+    virBitmapPtr ret = NULL;
>     char *tmp = NULL;
>+    VIR_AUTOPTR(virBitmap) def = NULL;
>
>     if (!(tmp = virXMLPropString(node, "cpuset"))) {
>         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>@@ -18412,14 +18406,14 @@ virDomainEmulatorPinDefParseXML(xmlNodePtr node)
>     if (virBitmapIsAllClear(def)) {
>         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                        _("Invalid value of 'cpuset': %s"), tmp);
>-        virBitmapFree(def);
>-        def = NULL;
>         goto cleanup;
>     }
>
>+    VIR_STEAL_PTR(ret, def);
>+

This 'ret' addition can also be separate.

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] conf: Use VIR_AUTOPTR(virBitmap) in domain_conf
Posted by John Ferlan 6 years, 11 months ago

On 2/20/19 10:37 AM, Ján Tomko wrote:
> On Wed, Feb 20, 2019 at 09:46:57AM -0500, John Ferlan wrote:
>> Let's make use of the auto __cleanup capabilities cleaning up any
>> now unnecessary goto paths.
> 
> My clang 7 and gcc 8.2 both happily compile code where the cleanup:
> label only contains a return statement, the goto paths can be cleaned
> up later.
> 

OK so new rules for domain_conf that didn't apply for the storage
changes....  OK - I will separate.

John

>>
>> Signed-off-by: John Ferlan <jferlan@redhat.com>
>> ---
>> src/conf/domain_conf.c | 53 ++++++++++++++++--------------------------
>> 1 file changed, 20 insertions(+), 33 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index ceeb247ef4..ddcb76f05d 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -1803,8 +1803,8 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr
>> def,
>>                                  virBitmapPtr autoCpuset)
>> {
>>     int maxvcpus = virDomainDefGetVcpusMax(def);
>> -    virBitmapPtr allcpumap = NULL;
>>     size_t i;
>> +    VIR_AUTOPTR(virBitmap) allcpumap = NULL;
>>
>>     if (hostcpus < 0)
>>         return -1;
>> @@ -1831,7 +1831,6 @@ virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr
>> def,
>>         virBitmapToDataBuf(bitmap, VIR_GET_CPUMAP(cpumaps, maplen, i),
>> maplen);
>>     }
>>
>> -    virBitmapFree(allcpumap);
>>     return i;
>> }
>>
>> @@ -2984,11 +2983,10 @@ static int
>> virDomainIOThreadIDDefArrayInit(virDomainDefPtr def,
>>                                 unsigned int iothreads)
>> {
>> -    int retval = -1;
>>     size_t i;
>>     ssize_t nxt = -1;
>>     virDomainIOThreadIDDefPtr iothrid = NULL;
>> -    virBitmapPtr thrmap = NULL;
>> +    VIR_AUTOPTR(virBitmap) thrmap = NULL;
>>
>>     /* Same value (either 0 or some number), then we have none to fill
>> in or
>>      * the iothreadid array was filled from the XML
>> @@ -2998,7 +2996,7 @@ virDomainIOThreadIDDefArrayInit(virDomainDefPtr
>> def,
>>
>>     /* iothread's are numbered starting at 1, account for that */
>>     if (!(thrmap = virBitmapNew(iothreads + 1)))
>> -        goto error;
>> +        return -1;
>>     virBitmapSetAll(thrmap);
>>
>>     /* Clear 0 since we don't use it, then mark those which are
>> @@ -3010,27 +3008,23 @@
>> virDomainIOThreadIDDefArrayInit(virDomainDefPtr def,
>>
>>     /* resize array */
>>     if (VIR_REALLOC_N(def->iothreadids, iothreads) < 0)
>> -        goto error;
>> +        return -1;
>>
>>     /* Populate iothreadids[] using the set bit number from thrmap */
>>     while (def->niothreadids < iothreads) {
>>         if ((nxt = virBitmapNextSetBit(thrmap, nxt)) < 0) {
>>             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>                            _("failed to populate iothreadids"));
>> -            goto error;
>> +            return -1;
>>         }
>>         if (VIR_ALLOC(iothrid) < 0)
>> -            goto error;
>> +            return -1;
>>         iothrid->iothread_id = nxt;
>>         iothrid->autofill = true;
>>         def->iothreadids[def->niothreadids++] = iothrid;
>>     }
>>
>> -    retval = 0;
>> -
>> - error:
>> -    virBitmapFree(thrmap);
>> -    return retval;
>> +    return 0;
>> }
>>
>>
>> @@ -18327,9 +18321,9 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node,
>> {
>>     int ret = -1;
>>     virDomainIOThreadIDDefPtr iothrid;
>> -    virBitmapPtr cpumask = NULL;
>>     unsigned int iothreadid;
>>     char *tmp = NULL;
>> +    VIR_AUTOPTR(virBitmap) cpumask = NULL;
>>
>>     if (!(tmp = virXMLPropString(node, "iothread"))) {
>>         virReportError(VIR_ERR_XML_ERROR, "%s",
>> @@ -18385,7 +18379,6 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node,
>>
>>  cleanup:
>>     VIR_FREE(tmp);
>> -    virBitmapFree(cpumask);
>>     return ret;
>> }
>>
>> @@ -18397,8 +18390,9 @@ virDomainIOThreadPinDefParseXML(xmlNodePtr node,
>> static virBitmapPtr
>> virDomainEmulatorPinDefParseXML(xmlNodePtr node)
>> {
>> -    virBitmapPtr def = NULL;
>> +    virBitmapPtr ret = NULL;
>>     char *tmp = NULL;
>> +    VIR_AUTOPTR(virBitmap) def = NULL;
>>
>>     if (!(tmp = virXMLPropString(node, "cpuset"))) {
>>         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> @@ -18412,14 +18406,14 @@ virDomainEmulatorPinDefParseXML(xmlNodePtr
>> node)
>>     if (virBitmapIsAllClear(def)) {
>>         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>>                        _("Invalid value of 'cpuset': %s"), tmp);
>> -        virBitmapFree(def);
>> -        def = NULL;
>>         goto cleanup;
>>     }
>>
>> +    VIR_STEAL_PTR(ret, def);
>> +
> 
> This 'ret' addition can also be separate.
> 
> Jano

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list