On Monday, May 5th, 2025 at 2:23 AM, Stefan Sperling <stsp@stsp.name> wrote:

> 
> 
> On Sun, May 04, 2025 at 08:47:27PM +0000, Kyle Ackerman wrote:
> 
> > The diff below patches gitconfig.c. When we are parsing a git config,
> > conf_parse_line frees *section before we allocate a new
> > section. This leaves the opportunity for *section to leak on the last
> > iteration of conf_parse.
> 
> > diff /home/kyle/src/got
> > path + /home/kyle/src/got
> > commit - 4492e47bc914650ecd587fcc94010ae0373ab91b
> > blob - 5af728df10a622bad125d0bc41ba804acfc9ccca
> > file + lib/gitconfig.c
> > --- lib/gitconfig.c
> > +++ lib/gitconfig.c
> > @@ -296,8 +296,10 @@ 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);
> > return err;
> 
> 
> Is this really a genuuine leak? I am asking because the only case
> where conf_parse_line() returns an error is when the allocation of
> *section has failed.

After looking at this again, you're correct. *section is not leaked if there
is an error.
> 
> > + }
> > line = cp + 1;
> > }
> > ln++;
> > @@ -306,6 +308,7 @@ conf_parse(struct got_gitconfig *conf, int trans, char
> > }
> > if (cp != line)
> > log_print("conf_parse: last line unterminated, ignored.");
> > + free(section);
> 
> 
> This chunk looks correct to me and fixes an obvious leak. ok stsp@
> 
> > return NULL;
> > }
> > 
> > Thoughts/Comments/Suggestions/oks?