apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Pane <bp...@pacbell.net>
Subject Re: apr_table_t (was: Re: Release time?)
Date Thu, 20 Sep 2001 04:48:30 GMT
Ian Holsman wrote:

>On Wed, 2001-09-19 at 14:28, Justin Erenkrantz wrote:
>>On Wed, Sep 19, 2001 at 12:25:36PM -0700, Brian Pane wrote:
>>>The original approach that I posted was a traditional iterator object:
>>>  typedef struct apr_table_iter_t apr_table_iter_t;
>>>  apr_table_iter_t * apr_table_iter_make(apr_pool_t *p,
>>>                                         const apr_table_t *t);
>>>  apr_status_t apr_table_iter_next(apr_table_iter_t *iterator,
>>>                                   const char **key,
>>>                                   const char **value);
>>>In my experience, this is an easy change to make, at least in the core
>>>modules (I only had to change half a dozen places in httpd-2.0 to get
>>>it to work.)
>>>Does anybody have thoughts for or against this iterator API?
>>+1.  Accessing the structures directly goes against our philosophy.
>Can we use the same API structure as the hash table?
>	apr_table_index_t* apr_table_first( apr_pool_t*p, apr_table_t*t)
>	apr_table_index_t* apr_table_next(apr_table_index_t*)
> 	void apr_table_this(apr_table_index_t*,const char**key,const
>			char**value)
Following the pattern of the apr_hash_t iterator is fine with me, too.

But I just noticed a small problem with the apr_hash_t iterator:

  APR_DECLARE(void) apr_hash_this(apr_hash_index_t *hi, const void **key,
                                  apr_ssize_t *klen, void **val);

The "val" is non-const.  Thus an app iterating through a hash table
can overwrite a value with NULL.  That's probably bad, as it violates
what otherwise is an invariant about apr_hash_t: values in it cannot
be NULL.  If you pass a NULL value to apr_hash_set(), it deletes the
key from the hash table, so it's ordinarily impossible to have a NULL
value in a hash table.

I propose that we fix this by either:
  * changing the hash iterator to a const iterator, or
  * documenting it.


View raw message