Re: [PATCH v2 07/34] sequencer (rebase -i): add support for the 'fixup' and 'squash' commands
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Dec 19, 2016, 17:14 UTC
- Message-ID
- <alpine.DEB.2.20.1612191812520.54750@virtualbox>
- In-Reply-To
- <20161215190351.as76panrcz5rgibj@sigill.intra.peff.net>
Hi Peff,
On Thu, 15 Dec 2016, Jeff King wrote:
Show 28 quoted lines
> On Tue, Dec 13, 2016 at 04:30:01PM +0100, Johannes Schindelin wrote:
>
> > + else {
> > + unsigned char head[20];
> > + struct commit *head_commit;
> > + const char *head_message, *body;
> > +
> > + if (get_sha1("HEAD", head))
> > + return error(_("need a HEAD to fixup"));
> > + if (!(head_commit = lookup_commit_reference(head)))
> > + return error(_("could not read HEAD"));
> > + if (!(head_message = get_commit_buffer(head_commit, NULL)))
> > + return error(_("could not read HEAD's commit message"));
>
> This get_commit_buffer() may allocate a fresh buffer...
>
> > + body = strstr(head_message, "\n\n");
> > + if (!body)
> > + body = "";
> > + else
> > + body = skip_blank_lines(body + 2);
> > + if (write_message(body, strlen(body),
> > + rebase_path_fixup_msg(), 0))
> > + return error(_("cannot write '%s'"),
> > + rebase_path_fixup_msg());
>
> ...and then this return leaks the result (the other code path hits
> unuse_commit_buffer(), and is fine).Good point.
I found another leaked commit buffer in make_patch() and fixed it, too.
> This leak was noticed by Coverity. It has a _ton_ of false positives > across the whole project, but it sends out a mail with new ones every > few days, which is usually short enough that I can process it in 30 > seconds or so.
Yeah, I get these mails now, thanks to Stephan adding me in response to some issues I introduced with the builtin difftool (and hence I did not get the warnings when I introduced the problems with sequencer-i).
Ciao, Dscho