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

Re: svn commit: r921716 - /subversion/trunk/subversion/libsvn_subr/mergeinfo.c

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Fri, 12 Mar 2010 11:55:27 +0000

Greg Stein wrote:
> On Thu, Mar 11, 2010 at 03:08, <julianfoad_at_apache.org> wrote:
> >...
> > +++ subversion/trunk/subversion/libsvn_subr/mergeinfo.c Thu Mar 11 08:08:06 2010
> >...
> > @@ -1311,23 +1309,20 @@ svn_mergeinfo_intersect2(svn_mergeinfo_t
> > for (hi = apr_hash_first(apr_hash_pool_get(mergeinfo1), mergeinfo1);
> > hi; hi = apr_hash_next(hi))
> > {
> > - apr_array_header_t *rangelist;
> > - const void *path;
> > - void *val;
> > - apr_hash_this(hi, &path, NULL, &val);
> > -
> > - rangelist = apr_hash_get(mergeinfo2, path, APR_HASH_KEY_STRING);
> > - if (rangelist)
> > - {
> > - SVN_ERR(svn_rangelist_intersect(&rangelist,
> > - (apr_array_header_t *) val,
> > - rangelist, consider_ineheritance,
> > - scratch_pool));
> > - if (rangelist->nelts > 0)
> > + const char *path = svn_apr_hash_index_key(hi);
> > + apr_array_header_t *rangelist1 = svn_apr_hash_index_val(hi);
> > + apr_array_header_t *rangelist2;
>
> const?

(throughout the file)

Yup, "const" would be better. But in order to do that we need to
constify the various functions in this part of the API, such as
svn_rangelist_dup().

I'll do a big sweeping change, constifying all apr_array_header_t *'s
that appear as inputs (i.e. logically const) in public and private APIs.
(Note that we are allowed to constify public APIs - and have done so
before - because that is a backward-compatible API change, and does not
change the ABI.)

> > +
> > + rangelist2 = apr_hash_get(mergeinfo2, path, APR_HASH_KEY_STRING);
>
> You could use svn_apr_hash_index_klen() rather than APR_HASH_KEY_STRING.

Yup, we could do that in all sorts of places in another change.

- Julian
Received on 2010-03-12 12:55:57 CET

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.