Re: [PATCH] Bug #45877: LONG_MAX/MIN array key as string/int

Hi Matt, I suppose I fixed the patch. Could you please check which patch is better yours or the attached one? According to attached benchmark my one is faster for most usual cases, but may be I forget something again. $a["abcdefghij"] 0.130 0.130 $a["1234567890"] 0.187 0.104 $a["2147483648"] 0.168 0.142 $a["0"] 0.116 0.082 $a["214748364x"] 0.136 0.141 $a[0] 0.080 0.080 Also, do you have other patches which I could look into before RC? Thanks. Dmitry. Matt Wilmas wrote: > Hi Dmitry, > > ----- Original Message ----- > From: "Dmitry Stogov" > Sent: Wednesday, March 18, 2009 > >> BTW may be we should eliminate strtol() at all. >> There's no need to scan the string twice. > > Your change to remove strtol() [1] is not checking for overflow > correctly (for example, zend_u_strtol()'s checks are more complicated). > This breaks handling of keys above ULONG_MAX (it's correct again after > ULONG_MAX+LONG_MAX, until ULONG_MAX * 2, etc.). See: > > var_dump(array('5000000000' => 1)); > > array(1) { > [705032704]=> > int(1) > } > > And of course the new code is a bit slower for keys that aren't fully > numeric, e.g. "123a" > > [1] http://news.php.net/php.zend-engine.cvs/7465 > >> Dmitry. > > - Matt > >> Matt Wilmas wrote: >>> Hi all, >>> >>> Assuming there are no objections, I'll commit this fix in a few hours... >>> >>> Besides the bug report(s), I had also found awhile ago that currently >>> an array key can be LONG_MAX or LONG_MIN as a string and/or integer >>> because of a check in ZEND_HANDLE_NUMERIC() (I assume to avoid a slow >>> errno check for ERANGE originally). I changed it to use the *same >>> method* that's used in the scanner, is_numeric_string(), etc., and >>> those 2 values are now treated as an integer. >>> >>> It's just a few lines changed in zend_hash.h, although I had to move >>> some of the definitions from zend_operators.h to zend.h (is that OK?). >>> >>> Patches (didn't check HEAD's yet): >>> http://realplain.com/php/array_key_limit.diff >>> http://realplain.com/php/array_key_limit_5_3.diff >>> >>> >>> - Matt >