Re: ZEND_HASH_APPLY_* confusion
| From: | Andi Gutmans | Date: | Sat, 04 Mar 2006 02:35:28 +0000 |
| Subject: | Re: ZEND_HASH_APPLY_* confusion | ||
| References: | 1 | Groups: | php.internals |
| Request: | Send a blank email to internals+get-22106@lists.php.net to get a copy of this message | ||
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<<1and 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