>>> On 17.03.16 at 15:37, <andrew.coop...@citrix.com> wrote: > On 17/03/16 11:49, Jan Beulich wrote: >>>>> On 15.03.16 at 20:33, <andrew.coop...@citrix.com> wrote: >>> On 15/03/16 17:56, Konrad Rzeszutek Wilk wrote: >>>> It looks like it could underflow at first glance. That is >>>> if i is zero and you get in the while loop with the >>>> i--. However the postfix expression is evaluated after the >>>> conditional so the loop is fine and won't execute (with i==0). >>>> >>>> However in spirit of defense programming lets clarify >>>> the loop conditional. >>>> >>>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com> >>> Reviewed-by: Andrew Cooper <andrew.coop...@citrix.com> >>> >>> This looks as if it will quieten Coverity, even though it is no >>> functional change. >> Quieten Coverity? In what way? And why would it complain in >> the first place? As just in reply to Konrad, this is well defined >> behavior. > > 213 error: > CID 63648: Overflowed constant (INTEGER_OVERFLOW) > 7. overflow_const: Decrement (--) operation overflows on operand > i, whose value is an unsigned constant, 0. > 214 while ( i-- ) > 215 free_domheap_page(mfn_to_page(mfn_x(mfn[i]))); > 216 xfree(mfn); > 217 return NULL; > > By flipping the location of the postfix decrement, the problematic case > of getting to error: with i as 0 will not enter the loop, and won't > decrement i to UINT32_MAX.
But (as alluded to before) this is a pretty common cleanup pattern, and I really don't see us (a) fix all instances just because Coverity complains and (b) avoid introducing any new instances. > It is arguable as to whether this is a Coverity bug or not. Unsigned > integer overflow is defined under the C spec. On the other hand, I > really don't blame Coverity for raising an issue here saying "did you > really mean for this underflow to happen". Since this is defined behavior, I personally view it as a Coverity issue. Which is not to say that such a warning may not help some people in certain cases. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
