On Tue, Mar 3, 2015 at 5:19 PM, Julian Foad <julianf...@btopenworld.com>
wrote:
> [Switching back to plain text]
>
> Stefan Fuhrmann wrote:
> > Julian Foad wrote:
> >> Thinking about how to prevent a repeat of the same kind of error,
> >> defining svn_hash_sets and svn_hash_gets as functions with prototypes
> >> would result in at least a compiler warning (for typical
> configurations).
> >
> > From r1484181 to r1509166, we already had various more sophisticated
> > definitions for those macros. Some of them would have caught the type
> > mismatch. I think it would be safe to change them into something like
> this:
> >
> > #define svn_hash_sets(ht, key, val) \
> > do { \
> > const char *svn_key__temp = (key); \
> > apr_hash_set(ht, svn_key__temp, strlen(svn_key__temp), val); \
> > } while (0)
>
> That would be safe, and works for *set*. We chose not to use strlen, so
> this would be
>
> #define svn_hash_sets(ht, key, val) \
> do { \
> const char *svn_hash__key = (key); \
> apr_hash_set(ht, svn_hash__key, APR_HASH_KEY_STRING, val); \
> } while (0)
>
But it would have caught the problem introduced in r1658194.
So, that would be a 50% solution with no obvious drawbacks.
> That style of definition doesn't work for *get*.
>
I can't think of a way to sneak in a type check into the getter
without evaluating the key twice.
In maintainer mode, though, we could replace the macro(s)
with a "proper" function. That would not produce overhead
for production code nor would it interfere with API guarantees.
-- Stefan^2.
