From: Junio C Hamano <junkio@cox.net>
Date: Fri, 02 Feb 2007 08:37:54 GMT
Subject: Re: [PATCH 1/2] Don't leak file descriptors from unavailable pack files.
Message-ID: <7vbqkdhs4d.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20070202080003.GA21094@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> If open_packed_git failed it may have been because the packfile
> actually exists and is readable, but some sort of verification
> did not pass.  In this case open_packed_git left pack_fd filled
> in, as the file descriptor is valid.

>  			if (p->pack_fd == -1 && open_packed_git(p)) {
> +				if (p->pack_fd != -1) {
> +					close(p->pack_fd);
> +					p->pack_fd = -1;
> +				}

I agree leaking fd is not nice, but I wonder if that should be
dealt with by the caller.

Originally it did not matter as open_packed_git() died, but
shouldn't it be closing the fd and marking p->pack_fd with -1, as
you made it return instead of die?