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

Re: [RFC] Simplify use of apr_hash_this()

From: Philip Martin <philip_at_codematters.co.uk>
Date: Tue, 30 Jun 2009 16:28:02 +0100

Julian Foad <julianfoad_at_btopenworld.com> writes:

This is gcc's "type-punned pointer will break strict-aliasing"
warning; I've never been able to decide whether this is a false alarm
or not. The gcc documentation states that false alarms may occur but
that there should be very few of them. However even if it is a false
alarm the code is suspect for another reason.

> 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);
> ...
> }

I don't think that's valid, although it will work on most common
platforms today. The problem is that in C a void* pointer can be
bigger than any particular data pointer. When the compiler assigns
to/from void* it adjusts for size as necessary, but the cast above
defeats that. So inside apr_hash_this a void* value will get written
into the space occupied by entry and that space might not be big

> +#if 1
> +#define APR_HASH_THIS(hi,key,klen,val) \
> + apr_hash_this((hi), (void *)(key), (klen), (void *)(val))

This is not valid for the reason above.

> +#else
> +#define APR_HASH_THIS(hi,key,klen,val) \
> + do { \
> + const void *__svn_key; \
> + apr_ssize_t __svn_klen; \
> + void *__svn_val; \
> + apr_hash_this((hi), &__svn_key, &__svn_klen, &__svn_val); \
> + if (key) *((const void **)key) = __svn_key; \
> + if (klen) *((apr_ssize_t *)klen) = __svn_klen; \
> + if (val) *((void **)val) = __svn_val; \
> + } while (0)
> +#endif

That's not valid because the casts in the three assignment lines have
the same problem. It should be OK without the casts.

Received on 2009-06-30 17:28:34 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.