Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:37345
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain realplain.com from 209.151.69.1 cause and error)
Message-ID: <00cf01c8aab0$0be2fb50$0201a8c0@pc1>
To: "Dmitry Stogov" <dmitry@zend.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>
Date: Wed, 30 Apr 2008 05:50:44 -0500
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [PATCH] More array filling optimizations
From: php_lists@realplain.com ("Matt Wilmas")

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...

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

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.