From: Stefan Sperling <stsp@stsp.name>
Subject: Re: fix 'gotadmin pack' when only tags are given
To: Christian Weisgerber <naddy@mips.inka.de>
Cc: gameoftrees@openbsd.org
Date: Sun, 29 Aug 2021 20:17:19 +0200

On Sun, Aug 29, 2021 at 04:38:22PM +0200, Christian Weisgerber wrote:
> Stefan Sperling:
> 
> > Fix 'gotadmin pack' not packing any objects if all arguments are tags.
> 
> Those "return err" in the changes and context should be "goto done".

Thanks, good point.

> The "GOT_OBJ_TYPE_TAG" and "else" branches perform the same actions.
> Would it be clearer to let the "GOT_OBJ_TYPE_TAG" code fall through?
> Like this:

There is a use after free here:

> @@ -688,7 +689,16 @@ findtwixt(struct got_object_id ***res, int *nres,
>  			continue;
>  		err = got_object_get_type(&obj_type, repo, id);
>  		if (err)
> -			return err;
> +			goto done;
> +		if (obj_type == GOT_OBJ_TYPE_TAG) {
> +			struct got_tag_object *tag;
> +			err = got_object_open_as_tag(&tag, repo, id);
> +			if (err)
> +				goto done;
> +			obj_type = got_object_tag_get_object_type(tag);
> +			id = got_object_tag_get_object_id(tag);

The above sets id = &tag->id

> +			got_object_tag_close(tag);

And this frees the tag.

Can this be fixed easily in your patch?
Essentially, queue_commit_id() must be called before got_object_tag_close().
Will you then end up with something that looks like the previous patch?