On 01/09/2013 11:55 AM, Laine Stump wrote:
(you duplicated "resource" in the subject line)
Missed that one... Will fix.
On 01/09/2013 09:54 AM, John Ferlan wrote:
Make cpuset local to the while loop and free it once done with it each time through the loop. --- src/xen/xend_internal.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 84a25e8..c6b800b 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1113,7 +1113,6 @@ sexpr_to_xend_topology(const struct sexpr *root, { const char *nodeToCpu; const char *cur; - virBitmapPtr cpuset = NULL; int *cpuNums = NULL; int cell, cpu, nb_cpus; int n = 0; @@ -1131,6 +1130,7 @@ sexpr_to_xend_topology(const struct sexpr *root,
cur = nodeToCpu; while (*cur != 0) { + virBitmapPtr cpuset = NULL; /* * Find the next NUMA cell described in the xend output */ @@ -1152,8 +1152,10 @@ sexpr_to_xend_topology(const struct sexpr *root, goto memory_error; } else { nb_cpus = virBitmapParse(cur, 'n', &cpuset, numCpus); - if (nb_cpus < 0) + if (nb_cpus < 0) { + virBitmapFree(cpuset);
This virBitmapFree() isn't necessary - virBitmapParse is guaranteed to have nothing allocated (and will set cpuset = NULL) if it fails.
According to Coverity's analysis this may not be true since it's "possible" to hit the "ret--" line (more than once) in virBitmapParse() while hitting either "ret++" line less times returning a negative value on the "success" path. The example Coverity had shows 6 passes through the loop, 4 negatives, 1 positive, and 1 nothing. Whether realistically this could be true, I am not sure. How Coverity determined what the value of 'cpuSet' is a mystery as the output I have doesn't show what's being used for parsing, just that we go through the loop 6 times. Perhaps something like "^1,^2,^3,4,^5,^6" where 1,2,3,4,5,6 pass the virBitmapIsSet() call changing the 'ret' value to -3.
goto error; + } }
for (n = 0, cpu = 0; cpu < numCpus; cpu++) { @@ -1163,28 +1165,26 @@ sexpr_to_xend_topology(const struct sexpr *root, if (used) cpuNums[n++] = cpu; } + virBitmapFree(cpuset);
if (virCapabilitiesAddHostNUMACell(caps, cell, nb_cpus, cpuNums) < 0) goto memory_error; + } VIR_FREE(cpuNums); - virBitmapFree(cpuset); return 0;
parse_error: virReportError(VIR_ERR_XEN_CALL, "%s", _("topology syntax error")); error: VIR_FREE(cpuNums); - virBitmapFree(cpuset); - return -1;
memory_error: VIR_FREE(cpuNums); - virBitmapFree(cpuset); virReportOOMError(); return -1; }
ACK with the above nits fixed.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list