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

From: Brandon Williams <bmwill@google.com>
Date: Dec 7, 2017, 19:44 UTC
Message-ID: <20171207194415.GE152943@google.com>
In-Reply-To: <20171205165854.64979-10-git@jeffhostetler.com>

On 12/05, Jeff Hostetler wrote:

> From: Jeff Hostetler <jeffhost@microsoft.com>
>

I'm a fan of eliminating looping goto statements. I understand their need for doing cleanup, but I think they should be reserved for that specific case. Thanks for cleaning this up!

Show 63 quoted lines

> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> ---
>  sha1_file.c | 40 ++++++++++++++++++++--------------------
>  1 file changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/sha1_file.c b/sha1_file.c
> index fc7718a..ce67f27 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1180,30 +1180,30 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,
>  		}
>  	}
>  
> -retry:
> -	if (find_pack_entry(real, &e))
> -		goto found_packed;
> +	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;
> +		/* 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))
> -		goto found_packed;
> -
> -	/* 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;
> -		goto retry;
> -	}
> +		/* Not a loose object; someone else may have just packed it. */
> +		reprepare_packed_git();
> +		if (find_pack_entry(real, &e))
> +			break;
>  
> -	return -1;
> +		/* 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;
> +	}
>  
> -found_packed:
>  	if (oi == &blank_oi)
>  		/*
>  		 * We know that the caller doesn't actually need the
> -- 
> 2.9.3
>

-- 
Brandon Williams