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

[RFC] Simplify use of apr_hash_this()

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Tue, 30 Jun 2009 13:05:50 +0100

I propose a new macro to simplify the use of apr_hash_this().

To de-reference an APR hash iterator, we want to write something like:

  for (hi = apr_hash_first(pool, entries); hi; hi = apr_hash_next(hi))
    {
      const char *item;
      svn_wc_entry_t *entry;

      apr_hash_this(hi, &item, NULL, &entry);

      ...
    }

But to avoid warnings or errors to do with pointer type conversions,
currently we have to either mess around with temporary variables:

  for (hi = apr_hash_first(pool, entries); hi; hi = apr_hash_next(hi))
    {
      const char *item;
      svn_wc_entry_t *entry;
      const void *key;
      void *val;

      apr_hash_this(hi, &key, NULL, &val);

      item = key;
      entry = val;

      ...
    }

which is what we normally do, or add explicit type casts:

  for (hi = apr_hash_first(pool, entries); hi; hi = apr_hash_next(hi))
    {
      const char *item;
      svn_wc_entry_t *entry;

      apr_hash_this(hi, (void *)&item, NULL, (void *)&entry);

      ...
    }

Whether such type casts make a detectable difference depends on what
compiler you're running, and so it is easy for us to get them wrong.
(Sometimes we write them with too much indirection: (void **), or with a
mis-placed "const".)

The attached patch "apr_hash_this-def.patch" allows us to write it as I
showed first, clean and simple. It provides a macro APR_HASH_THIS (we
can name it SVN_* if we think we should) that works like apr_hash_this()
except without the need for temporary variables or type casts at the
point of use.

The second attached patch "apr_hash_this-use.patch" demonstrates the use
of this macro in a few instances.

To me, this will enable a big improvement in code clarity. Anything
against it?

(The attached patch contains two alternative implementations, and I
haven't determined whether either one is sufficient, nor that they don't
produce any new warnings on my compiler, let alone other compilers. If
you are willing to test this, please let us know the result. If you want
to take this idea forward and tweak/commit/expand it, please do.
Otherwise I hope to get back to it at some point, but I'm putting it out
here while I concentrate on other things for an unknown time.)

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2366677

Received on 2009-06-30 14:06:27 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.