Hi Hannes,
On Thu, 27 Apr 2017, Johannes Sixt wrote:
> Am 26.04.2017 um 22:21 schrieb Johannes Schindelin:
> > We free()d the `log` buffer when dwim_log() returned 1, but not when it
> > returned a larger value (which meant that it still allocated the buffer
> > but we simply ignored it).
> >
> > Identified by Coverity.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > ---
> > reflog-walk.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/reflog-walk.c b/reflog-walk.c
> > index 99679f58255..ec66f2b16e6 100644
> > --- a/reflog-walk.c
> > +++ b/reflog-walk.c
> > @@ -183,7 +183,11 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
> > if (!reflogs || reflogs->nr == 0) {
> > struct object_id oid;
> > char *b;
> > - if (dwim_log(branch, strlen(branch), oid.hash, &b) ==
> > 1) {
> > + int ret = dwim_log(branch, strlen(branch),
> > + oid.hash, &b);
> > + if (ret > 1)
> > + free(b);
> > + else if (ret == 1) {
> > if (reflogs) {
> > free(reflogs->ref);
> > free(reflogs);
> >
>
> Right after this hunk, there is another conditional that looks like it
> forgets to free reflogs.
Thanks! Seems I got too hung up with the line to which Coverity pointed
and failed to see the bigger picture.
It seems that there are plenty of leaks, even further down. For one, the
`branch` variable is not released at the very end of the function! One
might think that read_complete_reflogs() takes custody of it, but no, it
xstrdup()s the refname right at the beginning.
So there was a lot more memory leaking going on in that function...
Ciao,
Dscho