Re: [PATCH 2/3] range-diff: handle commit ranges other than A..B

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
writes:

> From: Johannes Schindelin <johannes.schindelin@xxxxxx>
>
> In the `SPECIFYING RANGES` section of gitrevisions[7], two ways are
> described to specify commit ranges that `range-diff` does not yet
> accept: "<commit>^!" and "<commit>^-<n>".
>
> Let's accept them.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
>  builtin/range-diff.c  | 21 ++++++++++++++++++++-
>  t/t3206-range-diff.sh |  8 ++++++++
>  2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/range-diff.c b/builtin/range-diff.c
> index 551d3e689cb..6097635c432 100644
> --- a/builtin/range-diff.c
> +++ b/builtin/range-diff.c
> @@ -13,7 +13,26 @@ NULL
>  
>  static int is_range(const char *range)
>  {
> -	return !!strstr(range, "..");
> +	size_t i;
> +	char c;
> +
> +	if (strstr(range, ".."))
> +		return 1;
> +
> +	i = strlen(range);
> +	c = i ? range[--i] : 0;
> +	if (c == '!')
> +		i--; /* might be ...^! or ...^@ */

I am confused.  If it ends with '!', I do not see how it can end
with "^@".

If the input were "!", i gets strlen("!") which is 1, c gets '!'
while predecrementing i down to 0, and we notice c is '!' and
decrement i again to make it (size_t)(-1) which is a fairly large
number.

Then we skip all the else/if cascade, ensure that i is positive, and
happily access range[i], which likely is way out of bounds (but it
probably is almost one turn around the earth out of bounds, it may
access just a single byte before the array).

Am I reading the code right?

IOW, "git range-diff \! A..B" would do something strange, I would
guess.

> +	else if (isdigit(c)) {
> +		/* handle ...^-<n> */
> +		while (i > 2 && isdigit(range[--i]))
> +			; /* keep trimming trailing digits */
> +		if (i < 2 || range[i--] != '-')
> +			return 0;
> +	} else
> +		return 0;
> +
> +	return i > 0 && range[i] == '^';
>  }
>  
>  int cmd_range_diff(int argc, const char **argv, const char *prefix)
> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
> index 6eb344be031..e217cecac9e 100755
> --- a/t/t3206-range-diff.sh
> +++ b/t/t3206-range-diff.sh
> @@ -150,6 +150,14 @@ test_expect_success 'simple A B C (unmodified)' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'A^! and A^-<n> (unmodified)' '
> +	git range-diff --no-color topic^! unmodified^-1 >actual &&
> +	cat >expect <<-EOF &&
> +	1:  $(test_oid t4) = 1:  $(test_oid u4) s/12/B/
> +	EOF
> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'trivial reordering' '
>  	git range-diff --no-color master topic reordered >actual &&
>  	cat >expect <<-EOF &&



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux