Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:43441
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 212.25.124.163 as permitted sender)
Message-ID: <49C7A7FF.5090201@zend.com>
Date: Mon, 23 Mar 2009 18:17:19 +0300
User-Agent: Thunderbird 2.0.0.21 (X11/20090320)
MIME-Version: 1.0
To: Matt Wilmas <php_lists@realplain.com>
CC: internals@lists.php.net, Lukas Kahwe Smith <mls@pooteeweet.org>, 
 =?ISO-8859-1?Q?Johannes_Schl=FCter?= <johannes@php.net>
References: <1113CE12226949C2939A31971420991F@pc1> <49C0A7C7.8000804@zend.com> <12E613FAA1C9422B948F00B61AD32366@pc1> <49C134E5.7020706@zend.com> <570D833B543A4EB599F982488CDC41E4@pc1> <49C1E2ED.8080907@zend.com> <62A879A44B7A45AAB19266AB34ECDFA6@pc1> <49C39218.6000209@zend.com> <3E59E2234B594B84BBA2F22D9AEBDFFA@pc1> <49C799C5.6040509@zend.com> <5B65E362B45445E38CE1F652BC9FFB61@pc1>
In-Reply-To: <5B65E362B45445E38CE1F652BC9FFB61@pc1>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PATCH] Bug #45877: LONG_MAX/MIN array key as string/int
From: dmitry@zend.com (Dmitry Stogov)



Matt Wilmas wrote:
> Hi Dmitry,
> 
> ----- Original Message -----
> From: "Dmitry Stogov"
> Sent: Monday, March 23, 2009
> 
>> Hi Matt,
>>
>> Matt Wilmas wrote:
>>> Hi Dmitry,
>>>
>>> While updating my version (more below), I noticed another problem 
>>> that I overlooked before with your leading zero check: "-0" is being 
>>> treated as numeric.
>>
>> Yeah. Thank you for catching this. It should be fixed.
>>
>>> ----- Original Message -----
>>> From: "Dmitry Stogov"
>>> Sent: Friday, March 20, 2009
>>>
>>>> Hi Matt,
>>>>
>>>> I ran you version but it looked little bit slower (I checked it with 
>>>> callgrind). So I kept the current CVS version. Anyway, it is not a 
>>>> big problem to change it after RC.
>>>
>>> Yeah, after I sent that message, I noticed other times were strange 
>>> (bench.php, etc.), so it looks like I was getting compiler weirdness 
>>> with that code layout...  And of course looking at the code, there 
>>> shouldn't have been anything faster about it, though it was shorter, 
>>> which you confirmed with callgrind.
>>>
>>> Anyway, my updated version seems good, still smaller, and should do 
>>> better with callgrind. :-)  On 32-bit, the first digit of a 10 digit 
>>> number can be checked which avoids having "2x overflow" on 
>>> "5000000000", etc. that I pointed out with your original code.  (No 
>>> problem on 64-bit.) Also made the terminating null check be last 
>>> again since it's least likely to fail...
>>>
>>> http://realplain.com/php/handle_numeric.txt
>>> OLD: http://realplain.com/php/handle_numeric-v1.txt
>>
>> It looks like now your version must be better.
>> Are you sure that it can't miss overflow? Can it be proved?
> 
> Well, as far as I can tell, it can be proved. :-)  On 32-bit, <= 9 
> digits is obviously OK, and if there's 10, it's only used if it starts 
> with a 1 or 2. Overflow with 2500000000 is caught by the </> 0 check, 
> and 5000000000, which the </> 0 check in your first version missed, 
> won't be used since we can tell from the first digit that it wouldn't 
> fit.  On 64-bit, LONG_MAX starts with a 9, and there wouldn't be an 
> overflow missed by the </> 0 check until ULONG_MAX, which is another 
> digit longer, and would be skipped by the "numbers too long" check.

OK. I'll test and ally your patch after RC.

Dmitry.