On 06/04/2015 11:13 AM, Peter Krempa wrote:
> On Thu, Jun 04, 2015 at 08:34:00 -0400, John Ferlan wrote:
>>
>>
>> On 06/04/2015 08:15 AM, Peter Krempa wrote:
>>> Refactor the code flow a bit more to clear coverity errors.
>>>
>>> Store the cpu count in an intermediate variable and reuse it rather than
>>> caluclating the index.
>>> ---
>>> src/util/virprocess.c | 11 +++++++----
>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
>>> index a38cb75..c07e5cd 100644
>>> --- a/src/util/virprocess.c
>>> +++ b/src/util/virprocess.c
>>> @@ -474,12 +474,14 @@ virProcessGetAffinity(pid_t pid)
>>> size_t i;
>>> cpu_set_t *mask;
>>> size_t masklen;
>>> + size_t ncpus;
>>> virBitmapPtr ret = NULL;
>>>
>>> # ifdef CPU_ALLOC
>>> /* 262144 cpus ought to be enough for anyone */
>>> - masklen = CPU_ALLOC_SIZE(1024 << 8);
>>> - mask = CPU_ALLOC(1024 << 8);
>>> + ncpus = 1024 << 8;
>>> + masklen = CPU_ALLOC_SIZE(ncpus);
>>> + mask = CPU_ALLOC(ncpus);
>>>
>>> if (!mask) {
>>> virReportOOMError();
>>> @@ -488,6 +490,7 @@ virProcessGetAffinity(pid_t pid)
>>>
>>> CPU_ZERO_S(masklen, mask);
>>> # else
>>> + ncpus = 1024;
>>> if (VIR_ALLOC(mask) < 0)
>>> return NULL;
>>>
>>> @@ -501,10 +504,10 @@ virProcessGetAffinity(pid_t pid)
>>> goto cleanup;
>>> }
>>>
>>> - if (!(ret = virBitmapNew(masklen * 8)))
>>> + if (!(ret = virBitmapNew(ncpus)))
>>> goto cleanup;
>>>
>>> - for (i = 0; i < masklen * 8; i++) {
>>> + for (i = 0; i < ncpus; i++) {
>>> # ifdef CPU_ALLOC
>>> if (CPU_ISSET_S(i, masklen, mask))
>>
>> ^^^ Coverity still complains here
>>
>> No real change since previous...
>
> Would you mind sharing the error after applying this patch?
>
> Peter
>
Sure (it's cut-n-paste)
471 virBitmapPtr
472 virProcessGetAffinity(pid_t pid)
473 {
474 size_t i;
475 cpu_set_t *mask;
476 size_t masklen;
477 size_t ncpus;
478 virBitmapPtr ret = NULL;
479
480 # ifdef CPU_ALLOC
481 /* 262144 cpus ought to be enough for anyone */
(1) Event assignment: Assigning: "ncpus" = "262144UL".
Also see events: [cond_at_most][assignment][overrun-local]
482 ncpus = 1024 << 8;
483 masklen = CPU_ALLOC_SIZE(ncpus);
484 mask = CPU_ALLOC(ncpus);
485
(2) Event cond_false: Condition "!mask", taking false branch
486 if (!mask) {
487 virReportOOMError();
488 return NULL;
(3) Event if_end: End of if statement
489 }
490
491 CPU_ZERO_S(masklen, mask);
492 # else
493 ncpus = 1024;
494 if (VIR_ALLOC(mask) < 0)
495 return NULL;
496
497 masklen = sizeof(*mask);
498 CPU_ZERO(mask);
499 # endif
500
(4) Event cond_false: Condition "sched_getaffinity(pid, masklen, mask) < 0", taking false branch
501 if (sched_getaffinity(pid, masklen, mask) < 0) {
502 virReportSystemError(errno,
503 _("cannot get CPU affinity of process %d"), pid);
504 goto cleanup;
(5) Event if_end: End of if statement
505 }
506
(6) Event cond_false: Condition "!(ret = virBitmapNew(ncpus))", taking false branch
507 if (!(ret = virBitmapNew(ncpus)))
(7) Event if_end: End of if statement
508 goto cleanup;
509
(8) Event cond_true: Condition "i < ncpus", taking true branch
(13) Event loop_begin: Jumped back to beginning of loop
(14) Event cond_true: Condition "i < ncpus", taking true branch
(15) Event cond_at_most: Checking "i < ncpus" implies that "i" may be up to 262143 on the true branch.
Also see events: [assignment][assignment][overrun-local]
510 for (i = 0; i < ncpus; i++) {
511 # ifdef CPU_ALLOC
(9) Event cond_true: Condition "__cpu / 8 < masklen", taking true branch
(10) Event cond_true: Condition "((__cpu_mask const *)mask->__bits[__cpu / (64UL /* 8 * sizeof (__cpu_mask) */)] & (1UL /* (__cpu_mask)1 */ << __cpu % (64UL /* 8 * sizeof (__cpu_mask) */))) != 0", taking true branch
(11) Event cond_true: Condition "({...})", taking true branch
(16) Event assignment: Assigning: "__cpu" = "i". The value of "__cpu" may now be up to 262143.
(17) Event cond_true: Condition "__cpu / 8 < masklen", taking true branch
(18) Event overrun-local: Overrunning array of 16 8-byte elements at element index 4095 (byte offset 32760) by dereferencing pointer "(__cpu_mask const *)mask->__bits + __cpu / 64UL".
Also see events: [assignment][cond_at_most]
512 if (CPU_ISSET_S(i, masklen, mask))
513 ignore_value(virBitmapSetBit(ret, i));
514 # else
515 if (CPU_ISSET(i, mask))
516 ignore_value(virBitmapSetBit(ret, i));
517 # endif
(12) Event loop: Jumping back to the beginning of the loop
518 }
519
520 cleanup:
521 # ifdef CPU_ALLOC
522 CPU_FREE(mask);
523 # else
524 VIR_FREE(mask);
525 # endif
526
527 return ret;
528 }
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list