re: ole32: fix wrong timeout check

[Dan Kegel]

>-        if ((dwTimeout != INFINITE) && (start_time + dwTimeout >= now))
>+        if ((dwTimeout != INFINITE) && (now >= start_time + dwTimeout))

>From time immemorial, it has been drummed into me
that one should always handle wraparound when
checking timers.  So something like
    DWORD delta = now - start_time;
    if (dwTimeout != INFINITE) && (delta < 0x80000000UL) && (delta > dwTimeout))
might be more correct... I used to always use
a signed variable for this, but C compilers
now treat overflow of signed variables as
a conceptual error, so the subtraction has to be unsigned,
There's probably a cleaner way to code this,
but doing the subtraction to get the delta time,
and then comparing against the maximum desired
delta time, is the way to avoid trouble at rollover.
- Dan

ole32: fix wrong timeout check

[Alexander Dorofeyev]

Yeah these calculations are a headache. I could probably write

now < start_time || now >= start_time + dwTimeout

to at least avoid a "hang" if GetTickCount counter overflows. This won't solve 
not waiting the required time if, say, start_time + dwTimeout overflows, and 
that can also be a problem (for example, I found this bug by bisecting a 
regression in an installer that happened because the function didn't properly 
wait). To be really purist, we would probably need a macro or a function with 
branching that correctly handles all possible overflow situations and tells if 
the required interval elapsed between "tick" values A and B. I wonder if a patch 
like that would be accepted, though, and also in what file this macro/function 
could be implemented?

This seems to be pretty widespread, too, one of CoWaitForMultipleHandles callers 
- RPC_GetLocalClassObject - now also has a similar timeout check.

Dan Kegel wrote:
>> -        if ((dwTimeout != INFINITE) && (start_time + dwTimeout >= now))
>> +        if ((dwTimeout != INFINITE) && (now >= start_time + dwTimeout))
> 
>>From time immemorial, it has been drummed into me
> that one should always handle wraparound when
> checking timers.  So something like
>     DWORD delta = now - start_time;
>     if (dwTimeout != INFINITE) && (delta < 0x80000000UL) && (delta > dwTimeout))
> might be more correct... I used to always use
> a signed variable for this, but C compilers
> now treat overflow of signed variables as
> a conceptual error, so the subtraction has to be unsigned,
> There's probably a cleaner way to code this,
> but doing the subtraction to get the delta time,
> and then comparing against the maximum desired
> delta time, is the way to avoid trouble at rollover.
> - Dan
> 
>

ole32: fix wrong timeout check

[Alexandre Julliard]

Alexander Dorofeyev <alexd4(a)inbox.lv> writes:
> Yeah these calculations are a headache. I could probably write
>
> now < start_time || now >= start_time + dwTimeout

The right way is to write

 now - start_time > dwTimeout

using unsigned variables everywhere. Then you don't have to worry about
overflows, and you don't even need to special case INFINITE.

-- 
Alexandre Julliard
julliard(a)winehq.org