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

[Johannes Sixt <j.sixt@viscovery.net>]

Am 11/1/2010 23:54, schrieb Shawn O. Pearce:
> -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