On 06/02/2014 07:40 AM, Alexander Burluka wrote:
> From: A.Burluka <aburluka@xxxxxxxxxxxxx>
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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list