You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2017/07/12 10:53:32 UTC

svn commit: r1801722 - in /subversion/trunk/subversion: include/private/svn_mergeinfo_private.h libsvn_subr/deprecated.c libsvn_subr/mergeinfo.c tests/libsvn_subr/mergeinfo-test.c

Author: rhuijben
Date: Wed Jul 12 10:53:32 2017
New Revision: 1801722

URL: http://svn.apache.org/viewvc?rev=1801722&view=rev
Log:
Expose the mergerange canonical form helper functions in the private api
to allow hiding some implementation details. Improve the canonical form
api to verify assumptions related to inheritability.

This is initial plumbing for fixing issue #4686.

* subversion/include/private/svn_mergeinfo_private.h
  (svn_rangelist__parse): Update comment.
  (svn_rangelist__is_canonical): New function.
  (svn_rangelist__combine_adjacent_ranges): Remove function, as users
    should really call the canonicalize function.

* subversion/libsvn_subr/deprecated.c
  (svn_rangelist_merge): Canonicalize the result, instead of just merging
    ranges.

* subversion/libsvn_subr/mergeinfo.c
  (is_rangelist_normalized): Rename to...
  (svn_rangelist__is_canonical): ... this and remove the existing
     limitations.

  (svn_rangelist__canonicalize): Update caller and fold in the actual
     canonicalization that used to be in...
  (svn_rangelist__combine_adjacent_ranges): ... this function.

  (svn_rangelist_merge2): In SVN_DEBUG mode verify that the result is
     canonical when the input arguments are. Currently this assertion
     fails for issue #4686.

* subversion/tests/libsvn_subr/mergeinfo-test.c
  (test_rangelist_merge_overlap): Add some test assertions using the new
     helper function.

Modified:
    subversion/trunk/subversion/include/private/svn_mergeinfo_private.h
    subversion/trunk/subversion/libsvn_subr/deprecated.c
    subversion/trunk/subversion/libsvn_subr/mergeinfo.c
    subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-test.c

Modified: subversion/trunk/subversion/include/private/svn_mergeinfo_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_mergeinfo_private.h?rev=1801722&r1=1801721&r2=1801722&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_mergeinfo_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_mergeinfo_private.h Wed Jul 12 10:53:32 2017
@@ -52,18 +52,21 @@ svn_rangelist__set_inheritance(svn_range
  * Unlike svn_mergeinfo_parse(), this does not sort the ranges into order
  * or combine adjacent and overlapping ranges.
  *
- * The compaction can be done with svn_rangelist__combine_adjacent_ranges().
+ * The compaction can be done with svn_rangelist__canonicalize().
  */
 svn_error_t *
 svn_rangelist__parse(svn_rangelist_t **rangelist,
                      const char *str,
                      apr_pool_t *result_pool);
 
-/* In-place combines adjacent ranges in a rangelist.
-   SCRATCH_POOL is just used for providing error messages. */
-svn_error_t *
-svn_rangelist__combine_adjacent_ranges(svn_rangelist_t *rangelist,
-                                       apr_pool_t *scratch_pool);
+/* Return TRUE, if all ranges in RANGELIST are in ascending order and do
+* not overlap and are not adjacent.
+*
+* If this returns FALSE, you probaly want to call
+* svn_rangelist__canonicalize().
+*/
+svn_boolean_t
+svn_rangelist__is_canonical(const svn_rangelist_t *rangelist);
 
 /** Canonicalize the @a rangelist: sort the ranges, and combine adjacent or
  * overlapping ranges into single ranges where possible.

Modified: subversion/trunk/subversion/libsvn_subr/deprecated.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/deprecated.c?rev=1801722&r1=1801721&r2=1801722&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/deprecated.c (original)
+++ subversion/trunk/subversion/libsvn_subr/deprecated.c Wed Jul 12 10:53:32 2017
@@ -1288,7 +1288,7 @@ svn_rangelist_merge(svn_rangelist_t **ra
                                pool, pool));
 
   return svn_error_trace(
-            svn_rangelist__combine_adjacent_ranges(*rangelist, pool));
+            svn_rangelist__canonicalize(*rangelist, pool));
 }
 
 svn_error_t *

Modified: subversion/trunk/subversion/libsvn_subr/mergeinfo.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/mergeinfo.c?rev=1801722&r1=1801721&r2=1801722&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/mergeinfo.c (original)
+++ subversion/trunk/subversion/libsvn_subr/mergeinfo.c Wed Jul 12 10:53:32 2017
@@ -608,49 +608,48 @@ svn_rangelist__parse(svn_rangelist_t **r
   return SVN_NO_ERROR;
 }
 
-/* Return TRUE, if all ranges in RANGELIST are in ascending order and do
- * not overlap and are not adjacent.
- *
- * ### Can yield false negatives: ranges of differing inheritance are
- * allowed to be adjacent.
- *
- * If this returns FALSE, you probaly want to qsort() the
- * ranges and then call svn_rangelist__combine_adjacent_ranges().
- */
-static svn_boolean_t
-is_rangelist_normalized(svn_rangelist_t *rangelist)
+svn_boolean_t
+svn_rangelist__is_canonical(const svn_rangelist_t *rangelist)
 {
   int i;
   svn_merge_range_t **ranges = (svn_merge_range_t **)rangelist->elts;
 
-  for (i = 0; i < rangelist->nelts-1; ++i)
-    if (ranges[i]->end >= ranges[i+1]->start)
-      return FALSE;
-
-  return TRUE;
-}
-
-svn_error_t *
-svn_rangelist__canonicalize(svn_rangelist_t *rangelist,
-                            apr_pool_t *scratch_pool)
-{
-  if (! is_rangelist_normalized(rangelist))
+  /* Check for reversed and empty ranges */
+  for (i = 0; i < rangelist->nelts; ++i)
     {
-      svn_sort__array(rangelist, svn_sort_compare_ranges);
+      if (ranges[i]->start >= ranges[i]->end)
+        return FALSE;
+    }
 
-      SVN_ERR(svn_rangelist__combine_adjacent_ranges(rangelist, scratch_pool));
+  /* Check for overlapping ranges */
+  for (i = 0; i < rangelist->nelts - 1; ++i)
+    {
+      if (ranges[i]->end > ranges[i + 1]->start)
+        return FALSE; /* Overlapping range */
+      else if (ranges[i]->end == ranges[i+1]->start
+               && ranges[i]->inheritable == ranges[i + 1]->inheritable)
+        {
+          return FALSE; /* Ranges should have been combined */
+        }
     }
 
-  return SVN_NO_ERROR;
+  return TRUE;
 }
 
+/* In-place combines adjacent ranges in a rangelist.
+   SCRATCH_POOL is just used for providing error messages. */
 svn_error_t *
-svn_rangelist__combine_adjacent_ranges(svn_rangelist_t *rangelist,
-                                       apr_pool_t *scratch_pool)
+svn_rangelist__canonicalize(svn_rangelist_t *rangelist,
+                            apr_pool_t *scratch_pool)
 {
   int i;
   svn_merge_range_t *range, *lastrange;
 
+  if (svn_rangelist__is_canonical(rangelist))
+    return SVN_NO_ERROR; /* Nothing to do */
+
+  svn_sort__array(rangelist, svn_sort_compare_ranges);
+
   lastrange = APR_ARRAY_IDX(rangelist, 0, svn_merge_range_t *);
 
   for (i = 1; i < rangelist->nelts; i++)
@@ -964,6 +963,12 @@ svn_rangelist_merge2(svn_rangelist_t *ra
   int i = 0;
   int j = 0;
 
+#ifdef SVN_DEBUG
+  svn_boolean_t was_normalized =
+                    (svn_rangelist__is_canonical(rangelist)
+                     && svn_rangelist__is_canonical(changes));
+#endif
+
   /* We may modify CHANGES, so make a copy in SCRATCH_POOL. */
   changes = svn_rangelist_dup(changes, scratch_pool);
 
@@ -1189,6 +1194,10 @@ svn_rangelist_merge2(svn_rangelist_t *ra
       svn_sort__array_insert(rangelist, &change_copy, rangelist->nelts);
     }
 
+#ifdef SVN_DEBUG
+  SVN_ERR_ASSERT(!was_normalized || svn_rangelist__is_canonical(rangelist));
+#endif
+
   return SVN_NO_ERROR;
 }
 

Modified: subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-test.c?rev=1801722&r1=1801721&r2=1801722&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-test.c Wed Jul 12 10:53:32 2017
@@ -1738,7 +1738,12 @@ test_rangelist_merge_overlap(apr_pool_t
   }
 #endif
 
-  svn_rangelist_merge2(rangelist, changes, pool, pool);
+  SVN_TEST_ASSERT(svn_rangelist__is_canonical(rangelist));
+  SVN_TEST_ASSERT(svn_rangelist__is_canonical(changes));
+
+  SVN_ERR(svn_rangelist_merge2(rangelist, changes, pool, pool));
+
+  SVN_TEST_ASSERT(svn_rangelist__is_canonical(rangelist));
 
 #if 0
   {