Re: [PATCH 2/2] Work around EMFILE when there are too many pack files

From: Johannes Sixt <j.sixt@viscovery.net>
Date: Nov 2, 2010, 08:44 UTC
Message-ID: <4CCFCF8A.20608@viscovery.net>
In-Reply-To: <1288652061-19614-3-git-send-email-spearce@spearce.org>

Am 11/1/2010 23:54, schrieb Shawn O. Pearce:

Show 32 quoted lines

> -static int git_open_noatime(const char *name)
> +static int git_open_noatime(const char *name, struct packed_git *p)
>  {
>  	static int sha1_file_open_flag = O_NOATIME;
> -	int fd = open(name, O_RDONLY | sha1_file_open_flag);
>  
> -	/* Might the failure be due to O_NOATIME? */
> -	if (fd < 0 && errno != ENOENT && sha1_file_open_flag) {
> -		fd = open(name, O_RDONLY);
> +	for (;;) {
> +		int fd = open(name, O_RDONLY | sha1_file_open_flag);
>  		if (fd >= 0)
> +			return fd;
> +
> +		/* Might the failure be insufficient file descriptors? */
> +		if (errno == EMFILE) {
> +			if (unuse_one_window(p, -1))
> +				continue;
> +			else
> +				return -1;
> +		}
> +
> +		/* Might the failure be due to O_NOATIME? */
> +		if (errno != ENOENT && sha1_file_open_flag) {
>  			sha1_file_open_flag = 0;
> +			continue;
> +		}
> +
> +		return -1;
>  	}
> -	return fd;
>  }

Oh, boy! A function that returns a value, but does not have a return statement at the end? Even a goto would be easier to read:

retry:
	fd = open(name, O_RDONLY | sha1_file_open_flag);
	if (fd >= 0)
		return fd;

	if (errno == EMFILE) {
		if (unuse_one_window(p, -1))
			goto retry;
		return -1;
	}

	if (errno != ENOENT && sha1_file_open_flag) {
		sha1_file_open_flag = 0;
		goto retry;
	}

	return -1;

IMHO, of course.

-- Hannes