From: Omar Polo <op@omarpolo.com>
Subject: Re: gotwebd: plug leaks in got_get_repo_tags
To: Omar Polo <op@omarpolo.com>
Cc: Stefan Sperling <stsp@stsp.name>, gameoftrees@openbsd.org
Date: Fri, 02 Sep 2022 13:14:51 +0200

On 2022/09/02 12:45:34 +0200, Omar Polo <op@omarpolo.com> wrote:
> On 2022/09/02 11:42:49 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> > On Fri, Sep 02, 2022 at 11:31:31AM +0200, Omar Polo wrote:
> > > Here's an improved version of the diff i sent yesterday; now without
> > > leaks due to `continue'.  (there is still some work to do here wrt
> > > `id' and `commit_msg0', will do as follow-up diff)
> > 
> > Looks fine, ok stsp
> > 
> > It's quite hard to nagivate this jungle of loops, in the long term
> > we should consider splitting the function up into subroutines, as
> > this helps with leaks by scoping use of temp variables better.
> 
> +1
> 
> in fact, there was an issue with this diff
> 
> [..]
> 
> this fixes it:

since this was relatively easy to hit and it's a trivial diff, i
decided to go ahed and just commit it so we don't have known failures
in the -main branch.

> diff 5a57034b6f08eec784b8fe3c0b0b734eb2d4605a bc95141ca7ce90e4b19a251b36c87601c150bb3f
> commit - 5a57034b6f08eec784b8fe3c0b0b734eb2d4605a
> commit + bc95141ca7ce90e4b19a251b36c87601c150bb3f
> blob - 04cc799c9db1c0359740a967662340d9eb76f2cd
> blob + 4beb308778696b4c4299304065d186d28b072d3e
> --- gotwebd/got_operations.c
> +++ gotwebd/got_operations.c
> @@ -654,6 +654,8 @@ got_get_repo_tags(struct request *c, int limit)
>  		if (error)
>  			goto done;
>  
> +		if (tag)
> +			got_object_tag_close(tag);
>  		error = got_object_open_as_tag(&tag, repo, id);
>  		if (error) {
>  			if (error->code != GOT_ERR_OBJ_TYPE)
> @@ -686,9 +688,6 @@ got_get_repo_tags(struct request *c, int limit)
>  			    got_object_tag_get_object_id(tag));
>  			if (error)
>  				goto err;
> -
> -			got_object_tag_close(tag);
> -			tag = NULL;
>  		}
>  
>  		new_repo_tag->commit_id = strdup(id_str);