You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2010/03/11 09:08:07 UTC
svn commit: r921716 - /subversion/trunk/subversion/libsvn_subr/mergeinfo.c
Author: julianfoad
Date: Thu Mar 11 08:08:06 2010
New Revision: 921716
URL: http://svn.apache.org/viewvc?rev=921716&view=rev
Log:
Simplify hash indexing.
* subversion/libsvn_subr/mergeinfo.c
(svn_mergeinfo__set_inheritance, svn_mergeinfo_intersect2,
svn_mergeinfo_sort, svn_mergeinfo_catalog_dup, svn_mergeinfo_dup,
svn_mergeinfo_inheritable2,
svn_mergeinfo__remove_empty_rangelists,
svn_mergeinfo__remove_prefix_from_catalog,
svn_mergeinfo__get_range_endpoints,
svn_mergeinfo__filter_catalog_by_ranges,
svn_mergeinfo__filter_mergeinfo_by_ranges): Use svn_apr_hash_index_*()
instead of apr_hash_this(), giving variables the appropriate data type
and (in most cases) name, and avoiding type casts.
Modified:
subversion/trunk/subversion/libsvn_subr/mergeinfo.c
Modified: subversion/trunk/subversion/libsvn_subr/mergeinfo.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/mergeinfo.c?rev=921716&r1=921715&r2=921716&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/mergeinfo.c (original)
+++ subversion/trunk/subversion/libsvn_subr/mergeinfo.c Thu Mar 11 08:08:06 2010
@@ -849,9 +849,7 @@ svn_mergeinfo__set_inheritance(svn_merge
hi = apr_hash_next(hi))
{
apr_array_header_t *rangelist;
- const void *path;
- void *val;
- apr_hash_this(hi, &path, NULL, &val);
+ const char *path = svn_apr_hash_index_key(hi);
rangelist = apr_hash_get(mergeinfo, path, APR_HASH_KEY_STRING);
@@ -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;
+
+ rangelist2 = apr_hash_get(mergeinfo2, path, APR_HASH_KEY_STRING);
+ if (rangelist2)
+ {
+ SVN_ERR(svn_rangelist_intersect(&rangelist2, rangelist1, rangelist2,
+ consider_ineheritance, scratch_pool));
+ if (rangelist2->nelts > 0)
apr_hash_set(*mergeinfo,
apr_pstrdup(result_pool, path),
APR_HASH_KEY_STRING,
- svn_rangelist_dup(rangelist, result_pool));
+ svn_rangelist_dup(rangelist2, result_pool));
}
}
return SVN_NO_ERROR;
@@ -1447,14 +1442,11 @@ svn_error_t *
svn_mergeinfo_sort(svn_mergeinfo_t input, apr_pool_t *pool)
{
apr_hash_index_t *hi;
- void *val;
for (hi = apr_hash_first(pool, input); hi; hi = apr_hash_next(hi))
{
- apr_array_header_t *rl;
- apr_hash_this(hi, NULL, NULL, &val);
+ apr_array_header_t *rl = svn_apr_hash_index_val(hi);
- rl = val;
qsort(rl->elts, rl->nelts, rl->elt_size, svn_sort_compare_ranges);
}
return SVN_NO_ERROR;
@@ -1471,9 +1463,9 @@ svn_mergeinfo_catalog_dup(svn_mergeinfo_
hi;
hi = apr_hash_next(hi))
{
- const void *key;
- void *val;
- apr_hash_this(hi, &key, NULL, &val);
+ const char *key = svn_apr_hash_index_key(hi);
+ svn_mergeinfo_t val = svn_apr_hash_index_val(hi);
+
apr_hash_set(new_mergeinfo_catalog,
apr_pstrdup(pool, key),
APR_HASH_KEY_STRING,
@@ -1488,15 +1480,15 @@ svn_mergeinfo_dup(svn_mergeinfo_t mergei
{
svn_mergeinfo_t new_mergeinfo = apr_hash_make(pool);
apr_hash_index_t *hi;
- const void *path;
- apr_ssize_t pathlen;
- void *rangelist;
for (hi = apr_hash_first(pool, mergeinfo); hi; hi = apr_hash_next(hi))
{
- apr_hash_this(hi, &path, &pathlen, &rangelist);
+ const char *path = svn_apr_hash_index_key(hi);
+ apr_ssize_t pathlen = svn_apr_hash_index_klen(hi);
+ apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
+
apr_hash_set(new_mergeinfo, apr_pstrmemdup(pool, path, pathlen), pathlen,
- svn_rangelist_dup((apr_array_header_t *) rangelist, pool));
+ svn_rangelist_dup(rangelist, pool));
}
return new_mergeinfo;
@@ -1513,25 +1505,23 @@ svn_mergeinfo_inheritable2(svn_mergeinfo
apr_pool_t *scratch_pool)
{
apr_hash_index_t *hi;
- const void *key;
- apr_ssize_t keylen;
- void *rangelist;
-
svn_mergeinfo_t inheritable_mergeinfo = apr_hash_make(result_pool);
+
for (hi = apr_hash_first(scratch_pool, mergeinfo);
hi;
hi = apr_hash_next(hi))
{
+ const char *key = svn_apr_hash_index_key(hi);
+ apr_ssize_t keylen = svn_apr_hash_index_klen(hi);
+ apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
apr_array_header_t *inheritable_rangelist;
- apr_hash_this(hi, &key, &keylen, &rangelist);
- if (!path || svn_path_compare_paths(path, (const char *)key) == 0)
- SVN_ERR(svn_rangelist_inheritable2(&inheritable_rangelist,
- (apr_array_header_t *) rangelist,
+
+ if (!path || svn_path_compare_paths(path, key) == 0)
+ SVN_ERR(svn_rangelist_inheritable2(&inheritable_rangelist, rangelist,
start, end, inheritable,
result_pool, scratch_pool));
else
- inheritable_rangelist =
- svn_rangelist_dup((apr_array_header_t *)rangelist, result_pool);
+ inheritable_rangelist = svn_rangelist_dup(rangelist, result_pool);
/* Only add this rangelist if some ranges remain. A rangelist with
a path mapped to an empty rangelist is not syntactically valid */
@@ -1632,14 +1622,8 @@ svn_mergeinfo__remove_empty_rangelists(s
{
for (hi = apr_hash_first(pool, mergeinfo); hi; hi = apr_hash_next(hi))
{
- const void *key;
- void *value;
- const char *path;
- apr_array_header_t *rangelist;
-
- apr_hash_this(hi, &key, NULL, &value);
- path = key;
- rangelist = value;
+ const char *path = svn_apr_hash_index_key(hi);
+ apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
if (rangelist->nelts == 0)
{
@@ -1664,15 +1648,12 @@ svn_mergeinfo__remove_prefix_from_catalo
for (hi = apr_hash_first(pool, in_catalog); hi; hi = apr_hash_next(hi))
{
- const void *key;
- const char *original_path;
- void *value;
- apr_ssize_t klen;
+ const char *original_path = svn_apr_hash_index_key(hi);
+ apr_ssize_t klen = svn_apr_hash_index_klen(hi);
+ svn_mergeinfo_t *value = svn_apr_hash_index_val(hi);
- apr_hash_this(hi, &key, &klen, &value);
- original_path = key;
SVN_ERR_ASSERT(klen >= prefix_len);
- SVN_ERR_ASSERT(strncmp(key, prefix, prefix_len) == 0);
+ SVN_ERR_ASSERT(strncmp(original_path, prefix, prefix_len) == 0);
apr_hash_set(*out_catalog, original_path + prefix_len,
klen-prefix_len, value);
@@ -1859,14 +1840,7 @@ svn_mergeinfo__get_range_endpoints(svn_r
for (hi = apr_hash_first(pool, mergeinfo); hi; hi = apr_hash_next(hi))
{
- const void *key;
- void *value;
- const char *path;
- apr_array_header_t *rangelist;
-
- apr_hash_this(hi, &key, NULL, &value);
- path = key;
- rangelist = value;
+ apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
if (rangelist->nelts)
{
@@ -1903,14 +1877,10 @@ svn_mergeinfo__filter_catalog_by_ranges(
hi;
hi = apr_hash_next(hi))
{
- const void *key;
- void *val;
- const char *path;
-
- svn_mergeinfo_t mergeinfo, filtered_mergeinfo;
- apr_hash_this(hi, &key, NULL, &val);
- path = key;
- mergeinfo = val;
+ const char *path = svn_apr_hash_index_key(hi);
+ svn_mergeinfo_t mergeinfo = svn_apr_hash_index_val(hi);
+ svn_mergeinfo_t filtered_mergeinfo;
+
SVN_ERR(svn_mergeinfo__filter_mergeinfo_by_ranges(&filtered_mergeinfo,
mergeinfo,
youngest_rev,
@@ -1950,14 +1920,8 @@ svn_mergeinfo__filter_mergeinfo_by_range
hi;
hi = apr_hash_next(hi))
{
- const void *key;
- void *value;
- const char *path;
- apr_array_header_t *rangelist;
-
- apr_hash_this(hi, &key, NULL, &value);
- path = key;
- rangelist = value;
+ const char *path = svn_apr_hash_index_key(hi);
+ apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
if (rangelist->nelts)
{
Re: svn commit: r921716 -
/subversion/trunk/subversion/libsvn_subr/mergeinfo.c
Posted by Julian Foad <ju...@wandisco.com>.
I (Julian Foad) wrote:
> 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.)
r922239.
- Julian
Re: svn commit: r921716 -
/subversion/trunk/subversion/libsvn_subr/mergeinfo.c
Posted by Julian Foad <ju...@wandisco.com>.
Greg Stein wrote:
> On Thu, Mar 11, 2010 at 03:08, <ju...@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
Re: svn commit: r921716 - /subversion/trunk/subversion/libsvn_subr/mergeinfo.c
Posted by Greg Stein <gs...@gmail.com>.
On Thu, Mar 11, 2010 at 03:08, <ju...@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?
> +
> + rangelist2 = apr_hash_get(mergeinfo2, path, APR_HASH_KEY_STRING);
You could use svn_apr_hash_index_klen() rather than APR_HASH_KEY_STRING.
>...
> @@ -1488,15 +1480,15 @@ svn_mergeinfo_dup(svn_mergeinfo_t mergei
> {
> svn_mergeinfo_t new_mergeinfo = apr_hash_make(pool);
> apr_hash_index_t *hi;
> - const void *path;
> - apr_ssize_t pathlen;
> - void *rangelist;
>
> for (hi = apr_hash_first(pool, mergeinfo); hi; hi = apr_hash_next(hi))
> {
> - apr_hash_this(hi, &path, &pathlen, &rangelist);
> + const char *path = svn_apr_hash_index_key(hi);
> + apr_ssize_t pathlen = svn_apr_hash_index_klen(hi);
> + apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
const?
> +
> apr_hash_set(new_mergeinfo, apr_pstrmemdup(pool, path, pathlen), pathlen,
> - svn_rangelist_dup((apr_array_header_t *) rangelist, pool));
> + svn_rangelist_dup(rangelist, pool));
> }
>
> return new_mergeinfo;
> @@ -1513,25 +1505,23 @@ svn_mergeinfo_inheritable2(svn_mergeinfo
> apr_pool_t *scratch_pool)
> {
> apr_hash_index_t *hi;
> - const void *key;
> - apr_ssize_t keylen;
> - void *rangelist;
> -
> svn_mergeinfo_t inheritable_mergeinfo = apr_hash_make(result_pool);
> +
> for (hi = apr_hash_first(scratch_pool, mergeinfo);
> hi;
> hi = apr_hash_next(hi))
> {
> + const char *key = svn_apr_hash_index_key(hi);
> + apr_ssize_t keylen = svn_apr_hash_index_klen(hi);
> + apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
const?
>...
> @@ -1632,14 +1622,8 @@ svn_mergeinfo__remove_empty_rangelists(s
> {
> for (hi = apr_hash_first(pool, mergeinfo); hi; hi = apr_hash_next(hi))
> {
> - const void *key;
> - void *value;
> - const char *path;
> - apr_array_header_t *rangelist;
> -
> - apr_hash_this(hi, &key, NULL, &value);
> - path = key;
> - rangelist = value;
> + const char *path = svn_apr_hash_index_key(hi);
> + apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
const?
>...
> @@ -1859,14 +1840,7 @@ svn_mergeinfo__get_range_endpoints(svn_r
>
> for (hi = apr_hash_first(pool, mergeinfo); hi; hi = apr_hash_next(hi))
> {
> - const void *key;
> - void *value;
> - const char *path;
> - apr_array_header_t *rangelist;
> -
> - apr_hash_this(hi, &key, NULL, &value);
> - path = key;
> - rangelist = value;
> + apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
const?
>...
> @@ -1950,14 +1920,8 @@ svn_mergeinfo__filter_mergeinfo_by_range
> hi;
> hi = apr_hash_next(hi))
> {
> - const void *key;
> - void *value;
> - const char *path;
> - apr_array_header_t *rangelist;
> -
> - apr_hash_this(hi, &key, NULL, &value);
> - path = key;
> - rangelist = value;
> + const char *path = svn_apr_hash_index_key(hi);
> + apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
const?
Cheers,
-g
Re: svn commit: r921716 - /subversion/trunk/subversion/libsvn_subr/mergeinfo.c
Posted by Greg Stein <gs...@gmail.com>.
On Thu, Mar 11, 2010 at 03:08, <ju...@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?
> +
> + rangelist2 = apr_hash_get(mergeinfo2, path, APR_HASH_KEY_STRING);
You could use svn_apr_hash_index_klen() rather than APR_HASH_KEY_STRING.
>...
> @@ -1488,15 +1480,15 @@ svn_mergeinfo_dup(svn_mergeinfo_t mergei
> {
> svn_mergeinfo_t new_mergeinfo = apr_hash_make(pool);
> apr_hash_index_t *hi;
> - const void *path;
> - apr_ssize_t pathlen;
> - void *rangelist;
>
> for (hi = apr_hash_first(pool, mergeinfo); hi; hi = apr_hash_next(hi))
> {
> - apr_hash_this(hi, &path, &pathlen, &rangelist);
> + const char *path = svn_apr_hash_index_key(hi);
> + apr_ssize_t pathlen = svn_apr_hash_index_klen(hi);
> + apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
const?
> +
> apr_hash_set(new_mergeinfo, apr_pstrmemdup(pool, path, pathlen), pathlen,
> - svn_rangelist_dup((apr_array_header_t *) rangelist, pool));
> + svn_rangelist_dup(rangelist, pool));
> }
>
> return new_mergeinfo;
> @@ -1513,25 +1505,23 @@ svn_mergeinfo_inheritable2(svn_mergeinfo
> apr_pool_t *scratch_pool)
> {
> apr_hash_index_t *hi;
> - const void *key;
> - apr_ssize_t keylen;
> - void *rangelist;
> -
> svn_mergeinfo_t inheritable_mergeinfo = apr_hash_make(result_pool);
> +
> for (hi = apr_hash_first(scratch_pool, mergeinfo);
> hi;
> hi = apr_hash_next(hi))
> {
> + const char *key = svn_apr_hash_index_key(hi);
> + apr_ssize_t keylen = svn_apr_hash_index_klen(hi);
> + apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
const?
>...
> @@ -1632,14 +1622,8 @@ svn_mergeinfo__remove_empty_rangelists(s
> {
> for (hi = apr_hash_first(pool, mergeinfo); hi; hi = apr_hash_next(hi))
> {
> - const void *key;
> - void *value;
> - const char *path;
> - apr_array_header_t *rangelist;
> -
> - apr_hash_this(hi, &key, NULL, &value);
> - path = key;
> - rangelist = value;
> + const char *path = svn_apr_hash_index_key(hi);
> + apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
const?
>...
> @@ -1859,14 +1840,7 @@ svn_mergeinfo__get_range_endpoints(svn_r
>
> for (hi = apr_hash_first(pool, mergeinfo); hi; hi = apr_hash_next(hi))
> {
> - const void *key;
> - void *value;
> - const char *path;
> - apr_array_header_t *rangelist;
> -
> - apr_hash_this(hi, &key, NULL, &value);
> - path = key;
> - rangelist = value;
> + apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
const?
>...
> @@ -1950,14 +1920,8 @@ svn_mergeinfo__filter_mergeinfo_by_range
> hi;
> hi = apr_hash_next(hi))
> {
> - const void *key;
> - void *value;
> - const char *path;
> - apr_array_header_t *rangelist;
> -
> - apr_hash_this(hi, &key, NULL, &value);
> - path = key;
> - rangelist = value;
> + const char *path = svn_apr_hash_index_key(hi);
> + apr_array_header_t *rangelist = svn_apr_hash_index_val(hi);
const?
Cheers,
-g