Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:21728
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Message-ID: <000c01c62534$d48aef80$7d051fac@stumpy>
To: "Stefan Esser" <sesser@php.net>
Cc: <internals@lists.php.net>
References: <43DD2D20.5010001@php.net>
Date: Sun, 29 Jan 2006 16:33:43 -0800
MIME-Version: 1.0
Content-Type: text/plain;
	format=flowed;
	charset="iso-8859-1";
	reply-type=original
Content-Transfer-Encoding: 7bit
Subject: Re: zend_hash.c fishy code
From: pollita@php.net ("Sara Golemon")

> I wonder If I am completely missing the point, but the following piece
> of code seems fishy to me:
>
> ZEND_API int zend_hash_del_key_or_index(HashTable *ht, char *arKey, uint
> nKeyLength, ulong h, int flag)
> {
>        uint nIndex;
>        Bucket *p;
>
>       ...
>
>        while (p != NULL) {
>                if ((p->h == h) && ((p->nKeyLength == 0) || /* Numeric
> index */
>                        ((p->nKeyLength == nKeyLength) &&
> (!memcmp(p->arKey, arKey, nKeyLength))))) {
>                        HANDLE_BLOCK_INTERRUPTIONS();
>                         ...
>
> If I am not completely mistaken this means: If this is a bucket with a
> numeric index with the hash value of the key we want to delete, then
> delete it, even when we wanted to delete a key with a string as index.
>
There's a fairly important bit in that first ...:
    if (flag == HASH_DEL_KEY) {
        h = zend_inline_hash_func(arKey, nKeyLength);
    }

When deleting a numeric index nKeyLength is passed as zero and h contains 
the numeric index so this if only matches when p->h == h and p->nKeyLength 
== 0 (indicating a numericly keyed bucket).

When deleting a string index, nKeyLength is non-zero and h as passed is 
unimportant since it's overridden with the actual hash of arKey and 
nKeyLength.  The if then only evaluates true when the bucket hash matches 
the computed hash and the real key data in the bucket matches the real key 
data passed.

Yes, comparing p->h to h seems unnecessary since p->arKey is being compared 
to arKey anyway, but that seemingly redundant comparison saves a more costly 
memcmp() when the hashes differ.

-Sara