Re: [PATCH 2/2] range-diff: avoid negative string precision

From: Taylor Blau <me@ttaylorr.com>
Date: Apr 15, 2020, 16:20 UTC
Message-ID: <20200415162035.GD22823@syl.local>
In-Reply-To: <b3384880c7201d65adb7341ce23386d578e69193.1586960921.git.gitgitgadget@gmail.com>

On Wed, Apr 15, 2020 at 02:28:41PM +0000, Vasil Dimov via GitGitGadget wrote:

> From: Vasil Dimov <vd@FreeBSD.org>
>
> If the supplied integer for "precisoin" is negative in

s/precisoin/precision

Show 21 quoted lines

> `"%.*s", len, line` then it is ignored. So the current code is
> equivalent to just `"%s", line` because it is executed only if
> `len` is negative.
>
> Fix this by saving the value of `len` before overwriting it with the
> return value of `parse_git_diff_header()`.
>
> Signed-off-by: Vasil Dimov <vd@FreeBSD.org>
> ---
>  range-diff.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/range-diff.c b/range-diff.c
> index 5cc920be391..40af0862818 100644
> --- a/range-diff.c
> +++ b/range-diff.c
> @@ -123,16 +123,19 @@ static int read_patches(const char *range, struct string_list *list,
>  			struct patch patch = { 0 };
>  			struct strbuf root = STRBUF_INIT;
>  			int linenr = 0;
> +			int orig_len;

Any reason to not assign this to 'len' up here?

Show 9 quoted lines

>
>  			in_header = 0;
>  			strbuf_addch(&buf, '\n');
>  			if (!util->diff_offset)
>  				util->diff_offset = buf.len;
>  			line[len - 1] = '\n';
> +			orig_len = len;
>  			len = parse_git_diff_header(&root, &linenr, 0, line,
>  						    len, size, &patch);

OK, so we cut up the line by placing a NL at len, and then feed it to 'parse_git_diff_header' which will tell us the length of the thing that it parsed, or give a negative value if it couldn't parse...

>  			if (len < 0)
> -				die(_("could not parse git header '%.*s'"), (int)len, line);
> +				die(_("could not parse git header '%.*s'"),
> +				    orig_len, line);

...and then you restore the original length and print it out here. It seems like this error is now misleading though, because the line is already modified at the point that the newline was inserted.

Show 5 quoted lines

>  			strbuf_addstr(&buf, " ## ");
>  			if (patch.is_new > 0)
>  				strbuf_addf(&buf, "%s (new)", patch.new_name);
> --
> gitgitgadget

Thanks, Taylor