Re: [PATCH] double to long conversion change

Hi Matt, I tried to look into this issue once again, but I completely misunderstand why do we need all this magic. Why do we need conversion of positive double into negative long? I would stay with single DVAL_TO_LVAL() definition and use it in places instead of (long)Z_DVAL(). #define DVAL_TO_LVAL(d, l) \ if ((d) > LONG_MAX) { \ (l) = LONG_MAX; \ } else if ((d) < LONG_MIN) { \ (l) = LONG_MIN; \ } else {\ (l) = (long) (d); \ } Or may be we need a second macro for conversion into unsigned long where it needed? #define DVAL_TO_ULONG(d, l) \ if ((d) > ULONG_MAX) { \ (l) = ULONG_MAX; \ } else if ((d) < 0) { \ (l) = 0; \ } else {\ (l) = (unsigned long) (d); \ } It also possible to add notices in case of overflow detection. Thanks. Dmitry. Matt Wilmas wrote:

Hi all, Since noticing and reporting last year [1] different behavior when casting out-of-range doubles to int after the DVAL_TO_LVAL() macro was updated, I've wondered how to get the behavior I observed, and thought could be relied on (that was wrong to think, since it was un- or implementation-defined), back. And how to do so (what should be expected?), while keeping in mind the reason for the change: consistent behavior for tests. [2] Except that the current code does not give consistent results, depending on which DVAL_TO_LVAL definition is used on a platform. [3] [1] http://marc.info/?l=php-internals&m=120799720922202&w=2 [2] http://marc.info/?l=php-internals&m=123495655802226&w=2 [3] http://marc.info/?l=php-internals&m=123496364812725&w=2 So after I finally started to test my ideas for "consistent/reliable overflow across platforms" a few days ago, I noticed that my workaround technique quit working (0 instead of overflow) with doubles over 2^63, without resorting to fmod(). That's on Windows, but I suspect the same may happen on other systems that are limited to 64-bit integer processing internally or something (32-bit platforms?). On 64-bit Linux anyway, it looks like doubles > 2^63 do rollover as expected (128-bit "internal processing?"): http://marc.info/?l=php-internals&m=123376495021789&w=2 I wasn't sure how to rethink things after that... But of course with doubles, precision has been lost long before 2^63 anyway, as far as increments of 1 (it's 1024 at 2^63). What I wound up with for now, is using 5.2's method on 64-bit platforms, and on 32-bit, overflow behavior should be reliable up to 2^63 on platforms that have zend_long64 type available (long long, __int64), which I'm guessing is most (?), because of the unsigned long involvement. Finally a fallback workaround for 32-bit platforms without a 64-bit type. I updated a few other places in the code where only a (long) cast was used. And sort of unrelated, but I added an 'L' conversion specifier for zend_parse_parameters() in case it would be useful for PHP functions that want to limit values to LONG_MAX/LONG_MIN, without overflow, which I thought the DVAL_TO_LVAL change was *trying* to do. http://realplain.com/php/dval_to_lval.diff http://realplain.com/php/dval_to_lval_5_3.diff And here is an initial version of zend_dval_to_lval() (before 2^63 issue and thinking of zend_long64 + unsigned long), where some configure checks would set ZEND_DVAL_TO_LVAL_USE_* as needed. http://realplain.com/php/dval_to_lval.txt Any general feedback, comments, questions, suggestions? Hoping these conversion issues could be sorted out for good in a "nice," logical way. :-) Unfortunately on Windows, I'm just guessing, rather than testing, conversion results in different environments... Thanks, Matt