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

From: Junio C Hamano <[email-masked]>
Date: Tue, 05 Dec 2017 13:03:34 -0800

Jeff Hostetler <[email-masked]> writes:

> +	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.