On Sun, Feb 13, 2022 at 09:33:38PM +0100, Omar Polo wrote:
> Hello,
> 
> while studying a bit the code that wraps the various libexec helpers, I
> stumbled across this.  fetchibuf is local to the function and it doesn't
> seem to be used afterwards, same for imsg_fetchfds.  regress with the
> following diff passes, and I've also fetched from a couple of
> repositories successfully.
> 
> (the imsg_clear call is possibly useless if there aren't any pending
> messages enqueued, but I thought that it reads better; I can drop it)
> 
> 
> diff 32b5305fbed2d1a4c55b0eef6b93349b93ba7db0 /home/op/w/got-fetch-leak
> blob - 6c0dbf4a0ba8148e5f95f882f51bda4b71ebc4f1
> file + lib/fetch.c
> --- lib/fetch.c
> +++ lib/fetch.c
> @@ -353,6 +353,11 @@ got_fetch_pack(struct got_object_id **pack_hash, struc
>  			packfile_size = packfile_size_cur;
>  		}
>  	}
> +	imsg_clear(&fetchibuf);

Should the imsg not be cleared inside the above "while" loop body?
Just like it is done idxibuf on line 475 (line offset without your patch)?

Otherwise there is still a leak, isn't there?

> +	if (close(imsg_fetchfds[0]) == -1) {
> +		err = got_error_from_errno("close");
> +		goto done;
> +	}
>  	if (waitpid(fetchpid, &fetchstatus, 0) == -1) {
>  		err = got_error_from_errno("waitpid");
>  		goto done;
> 
>