]> xenbits.xensource.com Git - libvirt.git/commitdiff
virDomainFormatSchedDef: Avoid false positive NULL dereference
authorMichal Privoznik <mprivozn@redhat.com>
Thu, 2 Jun 2016 10:19:57 +0000 (12:19 +0200)
committerMichal Privoznik <mprivozn@redhat.com>
Thu, 2 Jun 2016 11:59:44 +0000 (13:59 +0200)
Okay, I admit that our code here is complex. It's not easy to
spot that NULL deref can't really happen here. So it's no wonder
that a dumb compiler fails to see all the connections and
produces the following errors:

  CC       conf/libvirt_conf_la-domain_conf.lo
conf/domain_conf.c: In function 'virDomainDefFormatInternal':
conf/domain_conf.c:22162:22: error: potential null pointer dereference [-Werror=null-dereference]
             if (sched->policy == i)
                 ~~~~~^~~~~~~~
<snip/>
cc1: all warnings being treated as errors

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
src/conf/domain_conf.c

index 568c699888f536033fd5addc98ccd736cb16c50b..7d46d0b3a32a4b9a535f1da499e659cbcdc12e2b 100644 (file)
@@ -22140,6 +22140,14 @@ virDomainFormatSchedDef(virDomainDefPtr def,
     size_t i;
     int ret = -1;
 
+    /* Okay, @func should never return NULL here because it does
+     * so iff corresponding resource does not exists. But if it
+     * doesn't we should not have been called in the first place.
+     * But some compilers fails to see this complex reasoning and
+     * deduct that this code is buggy. Shut them up by checking
+     * for return value of sched. Even though we don't need to.
+     */
+
     if (!(schedMap = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN)) ||
         !(prioMap = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN)))
         goto cleanup;
@@ -22152,7 +22160,7 @@ virDomainFormatSchedDef(virDomainDefPtr def,
         while ((next = virBitmapNextSetBit(resourceMap, next)) > -1) {
             sched = func(def, next);
 
-            if (sched->policy == i)
+            if (sched && sched->policy == i)
                 ignore_value(virBitmapSetBit(schedMap, next));
         }
 
@@ -22180,14 +22188,15 @@ virDomainFormatSchedDef(virDomainDefPtr def,
                 /* we need to find a subset of vCPUs with the given scheduler
                  * that share the priority */
                 nextprio = virBitmapNextSetBit(schedMap, -1);
-                sched = func(def, nextprio);
-                priority = sched->priority;
+                if (!(sched = func(def, nextprio)))
+                    goto cleanup;
 
+                priority = sched->priority;
                 ignore_value(virBitmapSetBit(prioMap, nextprio));
 
                 while ((nextprio = virBitmapNextSetBit(schedMap, nextprio)) > -1) {
                     sched = func(def, nextprio);
-                    if (sched->priority == priority)
+                    if (sched && sched->priority == priority)
                         ignore_value(virBitmapSetBit(prioMap, nextprio));
                 }