re: ole32: fix wrong timeout check
Dec. 22, 2007
3:01 a.m.
>- 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
December 2007
10:37 p.m.
New subject: ole32: fix wrong timeout check
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 > >
5:23 p.m.
New subject: ole32: fix wrong timeout check
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
6667
Age (days ago)
6668
Last active (days ago)
2 comments
3 participants
participants (3)
-
Alexander Dorofeyev -
Alexandre Julliard -
Dan Kegel