[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Valid values of svn_merge_range_t - no change number zero

From: Paul Burba <ptburba_at_gmail.com>
Date: Mon, 15 Mar 2010 14:45:10 -0400

On Fri, Mar 12, 2010 at 12:59 PM, Julian Foad <julian.foad_at_wandisco.com> wrote:
> Hi Paul.
>
> I think we can tighten the validation of svn_merge_range_t to exclude
> change number "r0" (RANGE->start == -1) as in the following patch.
>
> My reasoning is that a change numbered "r0" is not a valid concept in
> any Subversion system because the state (tree-snapshot) numbered r0 is
> by definition the beginning.  (It also happens to be empty by
> definition, but that's not so relevant.)  We can say the same in a
> different way: change "r0" would mean the change from "r(-1)" to "r0",
> and "r(-1)" is not a valid concept.
>
> Makes sense?

Hi Julian,

It does make sense, but I can't apply this patch, seems to have a few
problems, see below.

> [[[
> Tighten merge-range validation to not allow "change number r0" aka "revision
> range -1:Y".
>
> * 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.
> --This line, and those below, will be ignored--
>
> Index: subversion/tests/libsvn_subr/mergeinfo-test.c
> ===================================================================
> --- subversion/tests/libsvn_subr/mergeinfo-test.c       (revision 922300)
> +++ subversion/tests/libsvn_subr/mergeinfo-test.c       (working copy)
> @@ -877,24 +877,27 @@ test_remove_rangelist(apr_pool_t *pool)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Why is that all on one line? Those offsets are supposed to be on
their own line no?

Also, test_remove_rangelist(apr_pool_t *pool) looks to be on line 706
in https://svn.apache.org/repos/asf/subversion/trunk/subversion/tests/libsvn_subr/mergeinfo-test.c@922300,
so it seems your from-to headers are off.

>
>  #define RANDOM_REV_ARRAY_LENGTH 100
>
>  /* 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)
>  {
>   int i;
>   for (i = 0; i < RANDOM_REV_ARRAY_LENGTH; i++)
>     {
>       apr_uint32_t next = svn_test_rand(&random_rev_array_seed);
>       revs[i] = (next < 0x40000000) ? 0 : 1;
>     }
>  }
>
> +/* 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,
>                        apr_pool_t *pool)
>  {
>   svn_stringbuf_t *buf = svn_stringbuf_create("/trunk: ", pool);
> @@ -943,12 +946,15 @@ test_rangelist_remove_randomly(apr_pool_
                                                                ^^^
Again, all on one line and this time the line is truncated.

Anyhow, could try attaching the patch again?

Paul
Received on 2010-03-15 19:45:40 CET

This is an archived mail posted to the Subversion Dev mailing list.