Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:22106
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Message-ID: <7.0.1.0.2.20060303183504.066619a8@zend.com>
Date: Fri, 03 Mar 2006 18:35:28 -0800
To: Marcus Boerger <helly@php.net>,Derick Rethans <derick@derickrethans.nl>,
	Dmitry Stogov <dmitry@php.net>,Ilia Alshanetsky <ilia@prohost.org>,
	Rasmus Lerdorf <rasmus@lerdorf.com>,Zeev Suraski <zeev@zend.com>
Cc: internals@lists.php.net
In-Reply-To: <625666271.20060301202623@marcus-boerger.de>
References: <625666271.20060301202623@marcus-boerger.de>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; format=flowed
Subject: Re: ZEND_HASH_APPLY_* confusion
From: andi@zend.com (Andi Gutmans)

Yeah I think that making this consistent would have benefit.
Would the semantics be relevant to all methods you looked at?

Andi

At 11:26 AM 3/1/2006, Marcus Boerger wrote:
>Hello internals,
>
>   we have the consts:
>
>#define ZEND_HASH_APPLY_KEEP                            0
>#define ZEND_HASH_APPLY_REMOVE                          1<<0
>#define ZEND_HASH_APPLY_STOP                            1<<1
>
>and the apply functions:
>ZEND_API void zend_hash_apply(HashTable *ht, apply_func_t apply_func 
>TSRMLS_DC);
>ZEND_API void zend_hash_apply_with_argument(HashTable *ht, 
>apply_func_arg_t apply_func, void * TSRMLS_DC);
>ZEND_API void zend_hash_apply_with_arguments(HashTable *ht, 
>apply_func_args_t apply_func, int, ...);
>
>as well as:
>ZEND_API void zend_hash_reverse_apply(HashTable *ht, apply_func_t 
>apply_func TSRMLS_DC);
>
>we also have strange uscase:
>static int clean_non_persistent_constant(zend_constant *c TSRMLS_DC)
>{
>         if (c->flags & CONST_PERSISTENT) {
>                 return EG(full_tables_cleanup) ? 0 : ZEND_HASH_APPLY_STOP;
>         } else {
>                 return EG(full_tables_cleanup) ? 1 : ZEND_HASH_APPLY_REMOVE;
>         }
>}
>
>which actually would be the same as:
>static int clean_non_persistent_constant(zend_constant *c TSRMLS_DC)
>{
>         if (c->flags & CONST_PERSISTENT) {
>                 return EG(full_tables_cleanup) ? 
> ZEND_HASH_APPLY_KEEP : ZEND_HASH_APPLY_REMOVE;
>         } else {
>                 return ZEND_HASH_APPLY_REMOVE;
>         }
>}
>
>because normal apply functions only consider 0 and non 0.
>Only the revese apply function takes the STOP constant into account.
>Now obviously the case where a mixture of integres and consts is being
>used didn't lead to a problem but we should make them correct anyway.
>
>But i think we should also aim to make all apply functions aware of the
>STOP const becuase in some cases it would mean a speedup.
>
>Best regards,
>  Marcus