On 06/02/2014 07:40 AM, Alexander Burluka wrote:
From: A.Burluka <aburluka(a)parallels.com>
Subject line is WAAAY too long. You missed a blank line in between "add
domainGetVcpus()." and the rest of your commit message.
---
src/parallels/parallels_driver.c | 169 +++++++++++++++++++++++++++++++++++++-
src/parallels/parallels_utils.h | 1 +
2 files changed, 167 insertions(+), 3 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
+static int
+parallelsNodeGetCpuMask(parallelsDomObjPtr privatedomdata,
+ virBitmapPtr *cpumask,
+ int hostcpus)
+{
+ int ret = -1;
+ int cpunum = -1;
+ int prevcpunum = -1;
+ int offset = 0;
+ const char *it = privatedomdata->cpumask;
+ bool isrange = false;
+ size_t i;
+ int cpunums[512] = { 0 };
Why a magic number?
+ size_t count = 0;
+
+ if (STREQ(it, "all")) {
+ if (!(*cpumask = virBitmapNew(hostcpus)))
+ goto cleanup;
+ virBitmapSetAll(*cpumask);
+ } else {
+ while (sscanf(it, "%d%n", &cpunum, &offset)) {
sscanf(%d) is evil. It has undefined behavior on integer overflow, and
we have to assume that the input file that we are parsing could possibly
come from untrusted sources. Please use virstring.h virStrToLong_i()
instead.
+ case ',':
+ isrange = false;
+ break;
+ case '-':
+ isrange = true;
+ prevcpunum = cpunum;
+ break;
Instead of open-coding your own bitmap parser, can you see if
virBitmapParse() can do the job? If it can't, can it at least be
refactored into a common helper function with an additional parameter,
so that you can reuse that code?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org