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
enough.
> +#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.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2366745
Received on 2009-06-30 17:28:34 CEST