[libvirt] [PATCH v2 4/4] 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)
~~~~~^~~~~~~~
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(a)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