Re: [PATCH] msvcp120: Fix _Xtime_diff_to_millis2 overflow behavior.

[Piotr Caban]

On 10/11/17 17:07, Stefan Dösinger wrote:
> -
>    {0x7FFFFFFFFFFFFFFF, 0, 0x7FFFFFFFFFFFFFFD, 0, 0}, /* Not an overflow */

Defining LONGLONG constants this way is not portable.
> @@ -410,7 +410,13 @@ MSVCRT_long __cdecl _Xtime_diff_to_millis2(const xtime *t1, const xtime *t2)
>       diff_nsec = t1->nsec - t2->nsec;
>       ret = diff_sec * MILLISEC_PER_SEC +
>               (diff_nsec + NANOSEC_PER_MILLISEC - 1) / NANOSEC_PER_MILLISEC;
> - return ret > 0 ? ret : 0;
> -
> - /* This function does not return negative numbers, except when they get created through
> -
> * an integer overflow. */
> - if (diff_sec > ((LONGLONG)INT_MAX + NANOSEC_PER_MILLISEC - 1) / NANOSEC_PER_MILLISEC)
> -
>    return ret;
> - else
> -
>    return ret > 0 ? ret : 0;
> }

It looks like the function should be treating negative time differences 
in special way. Except of that it seems to be ignoring overflows. I 
think that following code is much easier to read. Is something like this 
working for you?
MSVCRT_long __cdecl _Xtime_diff_to_millis2(const xtime *t1, const xtime *t2)
{
     LONGLONG diff_sec, diff_nsec;

TRACE("(%p, %p)\n", t1, t2);

diff_sec = t1->sec - t2->sec;
     diff_nsec = t1->nsec - t2->nsec;

diff_sec += diff_nsec / NANOSEC_PER_SEC;
     diff_nsec %= NANOSEC_PER_SEC;
     if (diff_nsec < 0) {
         diff_sec -= 1;
         diff_nsec += NANOSEC_PER_SEC;
     }

if (diff_sec<0 || (diff_sec==0 && diff_nsec<0))
         return 0;
     return diff_sec * MILLISEC_PER_SEC +
         (diff_nsec + NANOSEC_PER_MILLISEC - 1) / NANOSEC_PER_MILLISEC;
}

Thanks,
Piotr