Re: [PATCHv4 3/4] bundle: don't leak an fd in case of early return

From

Junio C Hamano <gitster@pobox.com>

Date

Apr 1, 2016, 17:29 UTC

Message-ID

<xmqqk2khi6eo.fsf@gitster.mtv.corp.google.com>

In-Reply-To

<20160401171535.GB20345@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> You didn't see my response to the other patch yet. :)
>
>> >  	/* write pack */
>> >  	if (write_pack_data(bundle_fd, &revs))
>> > -		return -1;
>> > +		goto err;
>
> This does still suffer from the double-close I mentioned earlier. I'm
> not sure if we want to address that or not.

Something like this on top?

 bundle.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/bundle.c b/bundle.c
index 08e32ca..bbf4efa 100644
--- a/bundle.c
+++ b/bundle.c
@@ -453,8 +453,10 @@ int create_bundle(struct bundle_header *header, const char *path,
 		goto err;
 
 	/* write pack */
-	if (write_pack_data(bundle_fd, &revs))
+	if (write_pack_data(bundle_fd, &revs)) {
+		bundle_fd = -1; /* already closed by the above call */
 		goto err;
+	}
 
 	if (!bundle_to_stdout) {
 		if (commit_lock_file(&lock))
@@ -463,7 +465,8 @@ int create_bundle(struct bundle_header *header, const char *path,
 	return 0;
 err:
 	if (!bundle_to_stdout) {
-		close(bundle_fd);
+		if (0 <= bundle_fd)
+			close(bundle_fd);
 		rollback_lock_file(&lock);
 	}
 	return -1;