Re: [PATCH] log-tree: fix patch filename computation in "git format-patch"

[Junio C Hamano <gitster@pobox.com>]

Christian Couder <chriscool@tuxfamily.org> writes:

> When using "git format-patch", "get_patch_filename" in
> "log-tree.c" calls "strbuf_splice" that could die with
> the following message:
>
> "`pos + len' is too far after the end of the buffer"
>
> if you have:
>
> 	buf->len < start_len + FORMAT_PATCH_NAME_MAX
>
> but:
>
> 	buf->len + suffix_len > start_len + FORMAT_PATCH_NAME_MAX
>
> This patch tries to get rid of that bug.

hmm, tries to?

> diff --git a/log-tree.c b/log-tree.c
> index 56a3488..ade79ab 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -187,16 +187,17 @@ void get_patch_filename(struct commit *commit, int nr, const char *suffix,
>  
>  	strbuf_addf(buf, commit ? "%04d-" : "%d", nr);
>  	if (commit) {
> +		int max_len = start_len + FORMAT_PATCH_NAME_MAX;
>  		format_commit_message(commit, "%f", buf, DATE_NORMAL);
>  		/*
>  		 * Replace characters at the end with the suffix if the
>  		 * filename is too long
>  		 */
> +		if (buf->len + suffix_len > max_len) {
> +			int base = (max_len > buf->len) ? buf->len : max_len;
> +			strbuf_splice(buf, base - suffix_len, suffix_len,
> +				      suffix, suffix_len);
> +		} else
>  			strbuf_addstr(buf, suffix);

Your third argument to splice does not look right; if the existing length
is very very long, you would need to remove a lot, and if the existing
length is slightly long, you would need to remove just a little bit, but
you always seem to remove the fixed amount, to splice the suffix in.

In any case, why does this have to be so complex?

In your buffer, you originally have start_len, and would want to end up
with "%f" expansion, plus the suffix, but you are not allowed to exceed
FORMAT_PATCH_NAME_MAX to store what you add, and are only allowed to chop
the "%f" expansion if you are short of room.

Shouldn't it be just:

	size_t max_len = start_len + FORMAT_PATCH_NAME_MAX - suffix_len;
        if (max_len < buf->len)
                strbuf_setlen(buf, max_len);
	strbuf_addstr(buf, suffix);

The caller must make sure that suffix_len is sufficiently shorter than
FORMAT_PATCH_NAME_MAX; I do not know if the current code does that,
though.