Greg Stein wrote: 
> On Thu, Mar 11, 2010 at 03:08,  <julianf...@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


Reply via email to