Le vendredi 27 mars 2009, Junio C Hamano a écrit :
> Christian Couder <chriscool@xxxxxxxxxxxxx> 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?
Yeah, I was tired last night, when I created and sent this patch so I knew
that it could be wrong.
>
> > 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.
Yes, this looks better.
Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
