Author: julianfoad
Date: Wed Mar 17 09:37:11 2010
New Revision: 924201

URL: http://svn.apache.org/viewvc?rev=924201&view=rev
Log:
Tighten merge-range validation to not allow "change number r0" aka "revision
range -1:Y".

* subversion/libsvn_client/mergeinfo.c
  (filter_log_entry_with_rangelist): Ignore revision 0 because that doesn't
    represent a mergeable change.

* subversion/libsvn_subr/mergeinfo.c
  (IS_VALID_FORWARD_RANGE): New macro.
  (get_type_of_intersection): Use IS_VALID_FORWARD_RANGE() for tighter
    validation of arguments than before: it previously accepted "change 0".
  (range_intersect, range_contains): Validate arguments. Add doc strings.

* subversion/tests/libsvn_subr/mergeinfo-test.c
  (randomly_fill_rev_array, rev_array_to_rangelist): Expand doc strings.
  (test_rangelist_remove_randomly, test_rangelist_intersect_randomly): Don't
    ever include change number r0 in a merge range.

Modified:
    subversion/trunk/subversion/libsvn_client/mergeinfo.c
    subversion/trunk/subversion/libsvn_subr/mergeinfo.c
    subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-test.c

Modified: subversion/trunk/subversion/libsvn_client/mergeinfo.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/mergeinfo.c?rev=924201&r1=924200&r2=924201&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/mergeinfo.c (original)
+++ subversion/trunk/subversion/libsvn_client/mergeinfo.c Wed Mar 17 09:37:11 
2010
@@ -1296,7 +1296,12 @@ struct filter_log_entry_baton_t
 };
 
 /* Implements the svn_log_entry_receiver_t interface.  BATON is a
-   `struct filter_log_entry_baton_t *' */
+   `struct filter_log_entry_baton_t *'.
+
+   Call the wrapped log receiver BATON->log_receiver (with
+   BATON->log_receiver_baton), only if the log entry falls within the
+   ranges in BATON->rangelist.
+ */
 static svn_error_t *
 filter_log_entry_with_rangelist(void *baton,
                                 svn_log_entry_t *log_entry,
@@ -1308,6 +1313,10 @@ filter_log_entry_with_rangelist(void *ba
   if (fleb->ctx->cancel_func)
     SVN_ERR(fleb->ctx->cancel_func(fleb->ctx->cancel_baton));
 
+  /* Ignore r0 because there can be no "change 0" in a merge range. */
+  if (log_entry->revision == 0)
+    return SVN_NO_ERROR;
+
   this_rangelist = svn_rangelist__initialize(log_entry->revision - 1,
                                              log_entry->revision,
                                              TRUE, pool);
@@ -1436,7 +1445,7 @@ filter_log_entry_with_rangelist(void *ba
         }
     }
 
-  /* Call the wrapped log reveiver which this function is filtering for. */
+  /* Call the wrapped log receiver which this function is filtering for. */
   return fleb->log_receiver(fleb->log_receiver_baton, log_entry, pool);
 }
 

Modified: subversion/trunk/subversion/libsvn_subr/mergeinfo.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/mergeinfo.c?rev=924201&r1=924200&r2=924201&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/mergeinfo.c (original)
+++ subversion/trunk/subversion/libsvn_subr/mergeinfo.c Wed Mar 17 09:37:11 2010
@@ -115,6 +115,16 @@ parse_pathname(const char **input,
   return SVN_NO_ERROR;
 }
 
+/* Return TRUE iff (svn_merge_range_t *) RANGE describes a valid, forward
+ * revision range.
+ *
+ * Note: The smallest valid value of RANGE->start is 0 because it is an
+ * exclusive endpoint, being one less than the revision number of the first
+ * change described by the range, and the oldest possible change is "r1" as
+ * there cannot be a change "r0". */
+#define IS_VALID_FORWARD_RANGE(range) \
+  (SVN_IS_VALID_REVNUM((range)->start) && ((range)->start < (range)->end))
+
 /* Ways in which two svn_merge_range_t can intersect or adjoin, if at all. */
 typedef enum
 {
@@ -144,16 +154,8 @@ get_type_of_intersection(const svn_merge
 {
   SVN_ERR_ASSERT(r1);
   SVN_ERR_ASSERT(r2);
-
-  /* Why not use SVN_IS_VALID_REVNUM here?  Because revision 0
-     is described START = -1, END = 0.  See svn_merge_range_t. */
-  SVN_ERR_ASSERT(r1->start >= -1);
-  SVN_ERR_ASSERT(r2->start >= -1);
-
-  SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(r1->end));
-  SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(r2->end));
-  SVN_ERR_ASSERT(r1->start < r1->end);
-  SVN_ERR_ASSERT(r2->start < r2->end);
+  SVN_ERR_ASSERT(IS_VALID_FORWARD_RANGE(r1));
+  SVN_ERR_ASSERT(IS_VALID_FORWARD_RANGE(r2));
 
   if (!(r1->start <= r2->end && r2->start <= r1->end))
     *intersection_type = svn__no_intersection;
@@ -763,20 +765,31 @@ svn_rangelist_merge(apr_array_header_t *
   return SVN_NO_ERROR;
 }
 
+/* Return TRUE iff the forward revision ranges FIRST and SECOND overlap and
+ * (if CONSIDER_INHERITANCE is TRUE) have the same inheritability. */
 static svn_boolean_t
 range_intersect(const svn_merge_range_t *first, const svn_merge_range_t 
*second,
                 svn_boolean_t consider_inheritance)
 {
+  SVN_ERR_ASSERT_NO_RETURN(IS_VALID_FORWARD_RANGE(first));
+  SVN_ERR_ASSERT_NO_RETURN(IS_VALID_FORWARD_RANGE(second));
+
   return (first->start + 1 <= second->end)
     && (second->start + 1 <= first->end)
     && (!consider_inheritance
         || (!(first->inheritable) == !(second->inheritable)));
 }
 
+/* Return TRUE iff the forward revision range FIRST wholly contains the
+ * forward revision range SECOND and (if CONSIDER_INHERITANCE is TRUE) has
+ * the same inheritability. */
 static svn_boolean_t
 range_contains(const svn_merge_range_t *first, const svn_merge_range_t *second,
                svn_boolean_t consider_inheritance)
 {
+  SVN_ERR_ASSERT_NO_RETURN(IS_VALID_FORWARD_RANGE(first));
+  SVN_ERR_ASSERT_NO_RETURN(IS_VALID_FORWARD_RANGE(second));
+
   return (first->start <= second->start) && (second->end <= first->end)
     && (!consider_inheritance
         || (!(first->inheritable) == !(second->inheritable)));

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=924201&r1=924200&r2=924201&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-test.c Wed Mar 17 
09:37:11 2010
@@ -927,7 +927,8 @@ test_remove_rangelist(apr_pool_t *pool)
 /* Random number seed. */
 static apr_uint32_t random_rev_array_seed;
 
-/* Fill 3/4 of the array with 1s. */
+/* Set a random 3/4-ish of the elements of array REVS[RANDOM_REV_ARRAY_LENGTH]
+ * to TRUE and the rest to FALSE. */
 static void
 randomly_fill_rev_array(svn_boolean_t *revs)
 {
@@ -939,6 +940,8 @@ randomly_fill_rev_array(svn_boolean_t *r
     }
 }
 
+/* Set *RANGELIST to a rangelist representing the revisions that are marked
+ * with TRUE in the array REVS[RANDOM_REV_ARRAY_LENGTH]. */
 static svn_error_t *
 rev_array_to_rangelist(apr_array_header_t **rangelist,
                        svn_boolean_t *revs,
@@ -993,6 +996,9 @@ test_rangelist_remove_randomly(apr_pool_
 
       randomly_fill_rev_array(first_revs);
       randomly_fill_rev_array(second_revs);
+      /* There is no change numbered "r0" */
+      first_revs[0] = FALSE;
+      second_revs[0] = FALSE;
       for (j = 0; j < RANDOM_REV_ARRAY_LENGTH; j++)
         expected_revs[j] = second_revs[j] && !first_revs[j];
 
@@ -1048,6 +1054,9 @@ test_rangelist_intersect_randomly(apr_po
 
       randomly_fill_rev_array(first_revs);
       randomly_fill_rev_array(second_revs);
+      /* There is no change numbered "r0" */
+      first_revs[0] = FALSE;
+      second_revs[0] = FALSE;
       for (j = 0; j < RANDOM_REV_ARRAY_LENGTH; j++)
         expected_revs[j] = second_revs[j] && first_revs[j];
 


Reply via email to