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

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?
> 
>