Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:24840
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Message-ID: <017701c6acc6$3a526be0$0201a8c0@pc1>
To: <internals@lists.php.net>, <RQuadling@GoogleMail.com>
References: <014e01c6acbe$680b1bc0$0201a8c0@pc1> <10845a340607210543lfe173abq2221a1c833c5d9e1@mail.gmail.com>
Date: Fri, 21 Jul 2006 08:04:40 -0500
MIME-Version: 1.0
Content-Type: text/plain;
	charset="utf-8"
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [PATCH] array_combine is not binary-key safe
From: php_lists@realplain.com ("Matt W")

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


----- Original Message ----- 
From: "Richard Quadling"
Sent: Friday, July 21, 2006

> Hi,
>
> There are 46 uses of  add_assoc_zval() in the CVS. Many are with fixed
> length strings for the key. Should the others all be using
> add_assoc_zval_ex() ?
>
> On 21/07/06, Matt W <php_lists@realplain.com> wrote:
> > Hi,
> >
> > I don't know if array_combine() was intentionally made binary-key
unsafe,
> > but it seems wrong and inconsistent since binary keys work everywhere
else I
> > can think of -- including array_flip() and the new array_fill_keys().
And
> > updating it is a bit of an optimization by eliminating strlen() calls.
:-)
> >
> > I also changed the error message to be more grammatically-correct and
fixed
> > some typos in the description.
> >
> > http://www.realplain.com/php/array_combine_binkey.diff
> > http://www.realplain.com/php/array_combine_binkey_5_2.diff
> >
> >
> > Thanks,
> > Matt