[Spice-devel] [spice-server PATCH 2/3] test-loop: increment a variable outside of spice_assert

[Uri Lublin]

On 8/11/19 2:56 PM, Frediano Ziglio wrote:
>>
>> spice_assert is a macro and it may be that variable will
>> be incremented twice (in theory, possibly not in practice).
>>
> 
> No, the issue is that Coverity assume that code can be stripped out
> as usually assert can be stripped out (defining NDEBUG for normal
> assert).

You are correct that this is what the covscan complains about:
"The containing function might work differently in a non-debug build."

But spice_assert definition does not depend on NDEBUG.
On the other hand it does not happen in reality that
the variable is incremented twice.

I'll change the above comment to say
   spice_assert is a macro so prevent side-effects by
   not changing the variable in it.

Uri.

> 
>> Simply do it one line above.
>>
>> Found by covscan
>>
>> Signed-off-by: Uri Lublin <uril at redhat.com>


> 
> Beside the commit message no complains to make Coverity happy
> 
>> ---
>>   server/tests/test-loop.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/server/tests/test-loop.c b/server/tests/test-loop.c
>> index 82af80ab3..4df3a7d20 100644
>> --- a/server/tests/test-loop.c
>> +++ b/server/tests/test-loop.c
>> @@ -78,7 +78,8 @@ static SpiceTimer *twice_timers_remove[2] = { NULL, NULL };
>>   static int twice_remove_called = 0;
>>   static void timer_not_twice_remove(void *opaque)
>>   {
>> -    spice_assert(++twice_remove_called == 1);
>> +    ++twice_remove_called;
>> +    spice_assert(twice_remove_called == 1);
>>   
>>       /* delete timers, should not have another call */
>>       core->timer_remove(twice_timers_remove[0]);
> 
> Frediano
>