Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:37346
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 212.25.124.162 as permitted sender)
Message-ID: <48185599.1030308@zend.com>
Date: Wed, 30 Apr 2008 15:18:49 +0400
User-Agent: Thunderbird 2.0.0.12 (Windows/20080213)
MIME-Version: 1.0
To: Matt Wilmas <php_lists@realplain.com>
CC: internals@lists.php.net, Andi Gutmans <andi@zend.com>
References: <014b01c8a6db$57d53ba0$0201a8c0@pc1> <48181C02.3050705@zend.com> <00a701c8aaa9$e0fcd4c0$0201a8c0@pc1> <48184609.5000804@zend.com> <00cf01c8aab0$0be2fb50$0201a8c0@pc1>
In-Reply-To: <00cf01c8aab0$0be2fb50$0201a8c0@pc1>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [PATCH] More array filling optimizations
From: dmitry@zend.com (Dmitry Stogov)



Matt Wilmas wrote:
> Hi again Dmitry,
> 
> Hmm, if a -1 size was passed, I thought it would just make a huge array size
> (bad, obviously), but the tests would still work.  (Or I guess it might
> exhaust memory_limit.)  Anyway, the code for checking the size (length
> variable) in array_splice() is copied from php_splice(), so maybe there's
> some other error if it's becoming -1...

The bug causes memory overflow error, but the size definitely shouldn't 
be -1, as the same tests work fine before the patch.

> You don't think it makes sense to check return_value_used like
> next/prev/end/reset do?  They would only create a 1 element array, but
> array_splice() could waste time filling a large array that isn't used.  I
> think the function is used a lot to only remove/replace elements, not using
> the return_value:
> http://www.google.com/codesearch?q=lang%3Aphp+array_splice

Ah. I see. My mistake. Of course it makes sense.

Thanks. Dmitry.

> If there's a -1 size bug, the array_splice changes can be reverted and
> simply have:
> 
> if (return_value_used) {
>     /* Initialize return value */
>     array_init(return_value);
>     rem_hash = &Z_ARRVAL_P(return_value);
> }
> 
> - Matt
> 
> 
> ----- Original Message -----
> From: "Dmitry Stogov"
> Sent: Wednesday, April 30, 2008
> 
>> For some reason "make test" with the patch reported several broken
>> array_splice() tests. Looking in gdb I saw that init_array() got -1 as a
>> size of new array.
>>
>> I don't think checks for return_value_used for array_splice() have a lot
>> of sense.
>>
>> Thanks. Dmitry.
>