ext/sockets: further timeout handling changes.

[devnexen]

No description provided.

[cmb69]

Thank you!

[cmb69]

This looks not quite correct. If we get a large timeout (DWORD can be up to 2^32) and multiply by 1000, we can overflow (since unsigned no UB, but still not desireable, I think). Is there a reason not to do:

Suggested change

	tv.tv_usec = timeout ? (long)((timeout * 1000) % 1000000) : 0;
	tv.tv_usec = timeout ? (long)((timeout % 1000) * 1000) : 0;

[devnexen]

I did not change the windows part as I can't test (I should set up a php build in my windows laptop :)) but I ll have a look at your suggestion later.

[cmb69]

I didn't test either; and no need for Windows for this one. :)

#include <stdio.h>
#include <limits.h>

int main()
{
    unsigned int timeout = UINT_MAX;
    printf("%u\n", timeout);
    printf("%d\n", (long)((timeout * 1000) % 1000000));
    printf("%d\n", (long)((timeout % 1000) * 1000));
    return 0;
}

outputs:

4294967295
966296
295000

[devnexen]

fair fair, you got a point :)

[cmb69]

It seems to me that ULONG_MAX - (valusec / 1000) can yield a negative result, in which case we would throw. If we're fine with this (I think we are), the code is good as is.

[cmb69]

Maybe add a comment that valuesec / 1000 > ULONG_MAX is indeed considered a programmer error (i.e. throwing is deliberate here). Note that this code is only for Windows, where ULONG_MAX is 2^32, but ZEND_LONG_MAX is 2^63 for 64bit architectures.

[cmb69]

Looks good to me now.

[cmb69]

Maybe add a comment that valuesec / 1000 > ULONG_MAX is indeed considered a programmer error (i.e. throwing is deliberate here). Note that this code is only for Windows, where ULONG_MAX is 2^32, but ZEND_LONG_MAX is 2^63 for 64bit architectures.