Subject: Re: [PATCH v3 3/3] reftable/stack: make segment end inclusive

From: Junio C Hamano <[email-masked]>
Date: Fri, 29 Mar 2024 11:36:33 -0700

"Justin Tobler via GitGitGadget" <[email-masked]> writes:

> From: Justin Tobler <[email-masked]>
>
> For a reftable segment, the start of the range is inclusive and the end
> is exclusive. In practice we increment the end when creating the
> compaction segment only to decrement the segment end when using it.
>
> Simplify by making the segment end inclusive. The corresponding test,
> `test_suggest_compaction_segment()`, is updated to show that the segment
> end is now inclusive.
>
> Signed-off-by: Justin Tobler <[email-masked]>
> ---
>  reftable/stack.c      | 4 ++--
>  reftable/stack_test.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

I'd defer it to Patrick (and Han-Wen, if he wants to comment on it),
but isn't it a natural expectation shared among CS folks that it is
the most usual way to express a range to use inclusive lower-end and
exclusive upper-end?  

After all, that is how an array works, i.e. msg[n] is NULL and
beyond the end where n == strlen(msg).

So, I dunno.

> diff --git a/reftable/stack.c b/reftable/stack.c
> index e7b9a1de5a4..0973c47dd92 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -1237,7 +1237,7 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n)
>  	 */
>  	for (i = n - 1; i > 0; i--) {
>  		if (sizes[i - 1] < sizes[i] * 2) {
> -			seg.end = i + 1;
> +			seg.end = i;
>  			bytes = sizes[i];
>  			break;
>  		}

> @@ -1291,7 +1291,7 @@ int reftable_stack_auto_compact(struct reftable_stack *st)
>  		suggest_compaction_segment(sizes, st->merged->stack_len);
>  	reftable_free(sizes);
>  	if (segment_size(&seg) > 0)
> -		return stack_compact_range_stats(st, seg.start, seg.end - 1,
> +		return stack_compact_range_stats(st, seg.start, seg.end,
>  						 NULL);
>  
>  	return 0;
> diff --git a/reftable/stack_test.c b/reftable/stack_test.c
> index 21541742fe5..4d7305623a0 100644
> --- a/reftable/stack_test.c
> +++ b/reftable/stack_test.c
> @@ -723,7 +723,7 @@ static void test_suggest_compaction_segment(void)
>  	struct segment min =
>  		suggest_compaction_segment(sizes, ARRAY_SIZE(sizes));
>  	EXPECT(min.start == 1);
> -	EXPECT(min.end == 10);
> +	EXPECT(min.end == 9);
>  }
>  
>  static void test_suggest_compaction_segment_nothing(void)