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 }