Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:37890
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: <00a901c8bf1a$b46e6dd0$0201a8c0@pc1>
To: <internals@lists.php.net>,
	"Dmitry Stogov" <dmitry@zend.com>
References: <014b01c8a6db$57d53ba0$0201a8c0@pc1> <48181C02.3050705@zend.com> <00a701c8aaa9$e0fcd4c0$0201a8c0@pc1> <48184609.5000804@zend.com> <00cf01c8aab0$0be2fb50$0201a8c0@pc1> <48185599.1030308@zend.com>
Date: Mon, 26 May 2008 05:22:36 -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 Dmitry, all,

I looked into and fixed the -1 size and memory overflow error a couple weeks
or so ago, and finally updated the patches now. :-)  I made the mistake of
assuming the code that checks offset/length (in array_splice, etc.) set
length correctly, but it can still be negative, which broke
array_init_size...  Should be OK now there, and also in array_slice and
array_chunk.

So if it works correctly now, it sounded like the patch could be used?
(This doesn't have the compile/VM part, of course.)  I would be able to
commit these things myself now, but wouldn't unless I was given the OK
first! :-)

http://realplain.com/php/array_init_size.diff
http://realplain.com/php/array_init_size_5_3.diff


Thanks,
Matt


----- Original Message -----
From: "Dmitry Stogov"
Sent: Wednesday, April 30, 2008

>
> 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.