[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn_cache review

From: Eric Gillespie <epg_at_google.com>
Date: Tue, 25 Mar 2008 15:36:39 -0700

"David Glasser" <glasser_at_davidglasser.net> writes:

> > > Index: subversion/libsvn_subr/cache.c
> > > ===================================================================
> > > +struct svn_cache_t {
> > > + /* Maps from a key (of size CACHE->KLEN) to a struct cache_entry. */
> > > + apr_hash_t *hash;
> > > + apr_ssize_t klen;
> >
> > Maybe it doesn't matter, but why can keys be only one size?
> > apr_hash_get takes the key size, rather than storing it in the
> > apr_hash_t itself. This way you can use a string/length in
> > addition to a null-terminated string.
>
> Can you give me an example of any hash in the Subversion codebase that
> doesn't always use the same klen argument for every call to
> apr_hash_get and apr_hash_set?

Nope, I have no examples. Just wanted to see what you thought.

> > > +/* Uses CACHE->dup_func to copy VALUE into *VALUE_P inside POOL, or
> > > + just sets *VALUE_P to NULL if VALUE is NULL. */
> > > +static svn_error_t *
> > > +duplicate_value(void **value_p,
> > > + svn_cache_t *cache,
> > > + void *value,
> > > + apr_pool_t *pool)
> > > +{
> > > + if (value)
> > > + SVN_ERR((cache->dup_func)(value_p, value, pool));
> > > + else
> > > + *value_p = NULL;
> > > + return SVN_NO_ERROR;
> > > +}
> >
> > You take care to handle NULL values here, but apr_hash can't
> > handle them anyway, right? It uses a NULL value to remove a key.
>
> Right, but the values in the hash are cache_entry structs, whose
> 'value' member can be NULL. I've often found myself frustrated that
> APR hashes can't store NULL values; I didn't see any reason to
> continue that in this API.

Oh, of course; I see now.

Looks very nice!

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-03-25 23:37:43 CET

This is an archived mail posted to the Subversion Dev mailing list.