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.

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".

~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Reply via email to