Re: [PATCH 5/6] bundle: don't leak an fd in case of early return

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 29, 2016 at 05:38:52PM -0700, Stefan Beller wrote:

> In successful operation `write_pack_data` will close the `bundle_fd`,
> but when we exit early, we need to take care of the file descriptor
> ourselves.
> 
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
>  bundle.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/bundle.c b/bundle.c
> index 506ac49..04d62af 100644
> --- a/bundle.c
> +++ b/bundle.c
> @@ -434,8 +434,11 @@ int create_bundle(struct bundle_header *header, const char *path,
>  	init_revisions(&revs, NULL);
>  
>  	/* write prerequisites */
> -	if (compute_and_write_prerequisites(bundle_fd, &revs, argc, argv))
> +	if (compute_and_write_prerequisites(bundle_fd, &revs, argc, argv)) {
> +		if (!bundle_to_stdout)
> +			close(bundle_fd);
>  		return -1;
> +	}

Makes sense. Should we also be rolling back the lock file? It happens
automatically at program exit, of course, but we are in library code
here that should not rely on that.

> @@ -447,8 +450,11 @@ int create_bundle(struct bundle_header *header, const char *path,
>  	ref_count = write_bundle_refs(bundle_fd, &revs);
>  	if (!ref_count)
>  		die(_("Refusing to create empty bundle."));
> -	else if (ref_count < 0)
> +	else if (ref_count < 0) {
> +		if (!bundle_to_stdout)
> +			close(bundle_fd);
>  		return -1;
> +	}

Ditto here, and in the error return from write_pack_data(). There are
enough spots that it may be worth setting up a "goto err" path.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]