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