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

Re: Use svn_hash_gets

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Wed, 20 Mar 2013 15:16:10 +0000 (GMT)

C. Michael Pilato wrote:

> On 03/20/2013 09:45 AM, Julian Foad wrote:
>> Branko Čibej wrote:
>>> On 20.03.2013 04:47, Daniel Shahaf wrote:
>>>> See attached patch.  Any objections to doing that (and similar
>>>> changes to the rest of the code)?
>>>
>>> Must we do this just before branching? I suppose it's a benign change,
>>> but it /is/ a lot of code churn.
>>
>> When I tried doing this, I found I had to include <svn_hash.h> in every
>> C file that uses them, which is fairly close to every C file in the
>> project, and that made me think it would be better if we moved the
>> definitions into <svn_types.h> which is our "global" header file.

It would not be a big deal to include svn_hash.h in the files that we're
modifying.  However, these can be considered just convenience wrappers around APR hash functions, like svn__apr_hash_index_key/klen/val which are
already declared in svn_types.h so that they are globally available without
having to include svn_hash.h (since virtually all files include svn_types.h already).  It makes sense to me to do the same with
svn_hash_gets/sets.

This reminds me, I wonder if we're being premature in making these new functions public.

> Since svn_hash.h includes svn_types.h, won't this be more like replacing the
> inclusion of the latter with the inclusion of the former?

I'm not sure exactly what you mean, but if we do decide to leave the definitions in svn_hash.h and add '#include <svn_hash.h>' to each C file, that would be functionally equivalent to replacing one #include directive with the other, because of the include-guards.  I would oppose actually removing '#include <svn_types.h>' from the source files, as a matter of style.

>> Personally I don't mind the code churn and, for the purposes of
>> back-porting changes to 1.8.x, there is an advantage to doing it before
>> branching if we're going to do it at all (which I think we are).
>
> Speaking generally, I do mind the churn -- but this is, as was pointed out,
> a pretty benign change that requires no specialized domain knowledge to
> review.  I think it's "safe enough" to go ahead and pull the trigger.

- Julian
Received on 2013-03-20 16:16:45 CET

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