"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Kyle Ackerman <kack@kyleackerman.net>
Subject:
Re: Memoy Leak Patch
To:
Stefan Sperling <stsp@stsp.name>
Cc:
"gameoftrees@openbsd.org" <gameoftrees@openbsd.org>
Date:
Thu, 17 Apr 2025 13:59:22 +0000

Download raw body.

Thread

I will look at the first hunk again when I have time to see if there is a
better solution. 

Oks for the last two hunks to get in before release?


On Sunday, April 13th, 2025 at 10:16 AM, Stefan Sperling <stsp@stsp.name> wrote:

> 
> 
> On Sat, Apr 12, 2025 at 09:17:16PM +0000, Kyle Ackerman wrote:
> 
> > I found a couple of memory leaks to plug. I am currently working to
> > add more tests to our memleak regression
> > 
> > diff /home/kyle/src/got
> > path + /home/kyle/src/got
> > commit - 288f62bc8681fd4d88d574527fe45052a7d641b7
> > blob - 5af728df10a622bad125d0bc41ba804acfc9ccca
> > file + lib/gitconfig.c
> > --- lib/gitconfig.c
> > +++ lib/gitconfig.c
> > @@ -296,8 +296,11 @@ conf_parse(struct got_gitconfig *conf, int trans, char
> > *cp = '\0';
> > err = conf_parse_line(&section, conf, trans,
> > line, ln, cp - line);
> > - if (err)
> > + if (err) {
> > + free(section);
> 
> 
> This first chunk doesn't make sense to me because conf_parse_line()
> only returns an error if the allocation of the section failed. Or am
> I missing something?
> 
> Does your memleak report remain clean if you remove this chunk?
> 
> > return err;
> > + }
> > +
> > line = cp + 1;
> > }
> > ln++;
> 
> 
> The remaining changes below plug obvious leaks, and are ok by me.
> 
> > @@ -306,6 +309,7 @@ conf_parse(struct got_gitconfig *conf, int trans, char
> > }
> > if (cp != line)
> > log_print("conf_parse: last line unterminated, ignored.");
> > + free(section);
> > return NULL;
> > }
> > 
> > commit - 288f62bc8681fd4d88d574527fe45052a7d641b7
> > blob - 9bf6a20d6f6b7c2a585e7b921bd83e852fd59641
> > file + libexec/got-fetch-pack/got-fetch-pack.c
> > --- libexec/got-fetch-pack/got-fetch-pack.c
> > +++ libexec/got-fetch-pack/got-fetch-pack.c
> > @@ -849,6 +849,7 @@ done:
> > free(default_id_str);
> > free(refname);
> > free(server_capabilities);
> > + free(my_capabilities);
> > return err;
> > }
> > 
> > commit - 288f62bc8681fd4d88d574527fe45052a7d641b7
> > blob - 6fe7fc5ded32f612d6c9339abf7b9ae01aa06400
> > file + libexec/got-send-pack/got-send-pack.c
> > --- libexec/got-send-pack/got-send-pack.c
> > +++ libexec/got-send-pack/got-send-pack.c
> > @@ -606,6 +606,7 @@ done:
> > free(id);
> > free(refname);
> > free(server_capabilities);
> > + free(my_capabilities);
> > return err;
> > }
> > 
> > Thoughts/Comments/Suggestions?