[libvirt] [PATCH v2 4/4] virDomainFormatSchedDef: Avoid false positive NULL dereference

[Michal Privoznik]

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)
                 ~~~~~^~~~~~~~
conf/domain_conf.c:22191:26: error: potential null pointer dereference [-Werror=null-dereference]
                 priority = sched->priority;
                 ~~~~~~~~~^~~~~~~~~~~~~~~~~
conf/domain_conf.c:22197:30: error: potential null pointer dereference [-Werror=null-dereference]
                     if (sched->priority == priority)
                         ~~~~~^~~~~~~~~~
conf/domain_conf.c:22162:22: error: potential null pointer dereference [-Werror=null-dereference]
             if (sched->policy == i)
                 ~~~~~^~~~~~~~
conf/domain_conf.c:22191:26: error: potential null pointer dereference [-Werror=null-dereference]
                 priority = sched->priority;
                 ~~~~~~~~~^~~~~~~~~~~~~~~~~
conf/domain_conf.c:22197:30: error: potential null pointer dereference [-Werror=null-dereference]
                     if (sched->priority == priority)
                         ~~~~~^~~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/conf/domain_conf.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2efe0a3..e5a0e21 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22147,6 +22147,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;
@@ -22159,7 +22167,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));
         }
 
@@ -22187,14 +22195,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);
+                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));
                 }
 
-- 
2.8.3