Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:24873
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Message-ID: <00d401c6ad61$9d235810$0201a8c0@pc1>
To: <internals@lists.php.net>, "Andrei Zmievski" <andrei@gravitonic.com>
References: <014e01c6acbe$680b1bc0$0201a8c0@pc1> <10845a340607210543lfe173abq2221a1c833c5d9e1@mail.gmail.com> <017701c6acc6$3a526be0$0201a8c0@pc1> <FAE13B7E-4AA9-40C3-8907-90FA5A619B95@gravitonic.com>
Date: Sat, 22 Jul 2006 02:36:58 -0500
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [PATCH] array_combine is not binary-key safe
From: php_lists@realplain.com ("Matt W")

Hi Andrei,

I see you applied my patch.  However, the 5.2 code still isn't binary-key
safe (you only changed the second occurrence of add_assoc_zval to the _ex
version).  Or was that intentional and you only want to change the behavior
in 6?

And you know 5.2's description is still wrong -- with "keys" at the end
instead of "values"? :-)  When you changed that part in HEAD last week, you
also added a "the" -- "... as _the_ corresponding _values_" -- which was in
my patch, if you want both branches *exactly* the same.  :-P


Matt

P.S.  The other patch you're talking about below... I think Richard Quadling
said he'll do it.


----- Original Message -----
From: "Andrei Zmievski"
Sent: Friday, July 21, 2006


> Yeah, that's probably a good idea. You can submit a patch if you
> want. :)
>
> -Andrei
>
>
> On Jul 21, 2006, at 6:04 AM, Matt W wrote:
>
> > Hi Richard,
> >
> > I think I've seen those instances that you're referring to.  By
> > fixed length
> > string I assume you mean hard-coded "string_key".  Yeah, I would
> > think those
> > should use add_assoc_*_ex() since the length is known (sizeof
> > ("string_key")
> > etc.) to save unnecessary strlen() calls.
> >
> > Unless compilers optimize the strlen("string_key") + 1 to a
> > constant from
> > the add_assoc_*() macro.  But I wouldn't think that's the case...? :-/
> >
> >
> > Matt