From f916194c7ee9d212be1a1b2ff28625fca3e13aec Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Thu, 2 Jun 2016 12:19:57 +0200 Subject: [PATCH] virDomainFormatSchedDef: Avoid false positive NULL dereference 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) ~~~~~^~~~~~~~ cc1: all warnings being treated as errors Signed-off-by: Michal Privoznik --- 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 568c699888..7d46d0b3a3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -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)); }