Web lists-archives.com

Re: [PATCH v2 09/16] remote.c: turn some error() or die() to BUG()




Nguyễn Thái Ngọc Duy  <pclouds@xxxxxxxxx> writes:

> The first error, "internal error", is clearly a BUG(). The second two
> are meant to catch calls with invalid parameters and should never
> happen outside the test suite.

Sounds as if it would happen inside test suites.

I guess by "inside the test suite" you mean a test that links broken
callers to this code as if it is a piece of library function, but
"should never happen, even with invalid or malformed end-user
request" would convey the point better, as the normal part of
testing that is done by driing "git we just built from the command
line should not hit these.

The changes all look sensible.  Thanks.

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
>  remote.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index 81f4f01b00..257630ff21 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -620,7 +620,7 @@ static void handle_duplicate(struct ref *ref1, struct ref *ref2)
>  			 * FETCH_HEAD_IGNORE entries always appear at
>  			 * the end of the list.
>  			 */
> -			die(_("Internal error"));
> +			BUG("Internal error");
>  		}
>  	}
>  	free(ref2->peer_ref);
> @@ -707,7 +707,7 @@ static void query_refspecs_multiple(struct refspec *rs,
>  	int find_src = !query->src;
>  
>  	if (find_src && !query->dst)
> -		error("query_refspecs_multiple: need either src or dst");
> +		BUG("query_refspecs_multiple: need either src or dst");
>  
>  	for (i = 0; i < rs->nr; i++) {
>  		struct refspec_item *refspec = &rs->items[i];
> @@ -735,7 +735,7 @@ int query_refspecs(struct refspec *rs, struct refspec_item *query)
>  	char **result = find_src ? &query->src : &query->dst;
>  
>  	if (find_src && !query->dst)
> -		return error("query_refspecs: need either src or dst");
> +		BUG("query_refspecs: need either src or dst");
>  
>  	for (i = 0; i < rs->nr; i++) {
>  		struct refspec_item *refspec = &rs->items[i];