Re: [PATCH v6 09/12] fixup: sha1_file: convert gotos to break/continue

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 5, 2017, 21:03 UTC
Message-ID: <xmqqfu8oj3a1.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171205165854.64979-10-git@jeffhostetler.com>

Jeff Hostetler <git@jeffhostetler.com> writes:

Show 23 quoted lines

> +	while (1) {
> +		if (find_pack_entry(real, &e))
> +			break;
>  
> +		/* Most likely it's a loose object. */
> +		if (!sha1_loose_object_info(real, oi, flags))
> +			return 0;
>  
> +		/* Not a loose object; someone else may have just packed it. */
> +		reprepare_packed_git();
> +		if (find_pack_entry(real, &e))
> +			break;
>  
> +		/* Check if it is a missing object */
> +		if (fetch_if_missing && repository_format_partial_clone &&
> +		    !already_retried) {
> +			fetch_object(repository_format_partial_clone, real);
> +			already_retried = 1;
> +			continue;
> +		}
> +
> +		return -1;
> +	}

OK. This "infinite" loop actually never runs more than twice, thanks to "already-retried" thing. It probably makes sense to structure the code this way.

I suspect that this would also have worked better with the older incarnation of the jk/fewer-pack-rescan topic where an optimization used to be after that the initial queries to packs fail to find the object, but the topic has since been updated to do its optimization check much earlier in the function, so its interaction with this topic would not be much of an issue anymore with or without this rewrite.

In any case, I think this is a good change.