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

Re: svn commit: r1663450 - /subversion/trunk/subversion/libsvn_ra_svn/editorp.c

From: Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
Date: Tue, 3 Mar 2015 17:46:16 +0100

On Tue, Mar 3, 2015 at 5:19 PM, Julian Foad <julianfoad_at_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.
Received on 2015-03-03 17:46:48 CET

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