Re: [PATCH v4 2/3] remote.c: convert if-else ladder to switch

[Patrick Steinhardt <ps@pks.im>]

On Thu, Aug 07, 2025 at 09:41:11PM -0700, Denton Liu wrote:
> diff --git a/remote.c b/remote.c
> index 88f991795b..61e2c9951a 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1171,7 +1171,6 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,
>  						 const char *matched_src_name)
>  {
>  	struct object_id oid;
> -	enum object_type type;

> @@ -1196,28 +1195,33 @@ static void show_push_unqualified_ref_name_error(const char *dst_value,
>  		BUG("'%s' is not a valid object, "
>  		    "match_explicit_lhs() should catch this!",
>  		    matched_src_name);
> -	type = odb_read_object_info(the_repository->objects, &oid, NULL);
> -	if (type == OBJ_COMMIT) {
> +
> +	switch (odb_read_object_info(the_repository->objects, &oid, NULL)) {
> +	case OBJ_COMMIT:
>  		advise(_("The <src> part of the refspec is a commit object.\n"
>  			 "Did you mean to create a new branch by pushing to\n"
>  			 "'%s:refs/heads/%s'?"),
>  		       matched_src_name, dst_value);
> -	} else if (type == OBJ_TAG) {
> +		break;
> +	case OBJ_TAG:
>  		advise(_("The <src> part of the refspec is a tag object.\n"
>  			 "Did you mean to create a new tag by pushing to\n"
>  			 "'%s:refs/tags/%s'?"),
>  		       matched_src_name, dst_value);
> -	} else if (type == OBJ_TREE) {
> +		break;
> +	case OBJ_TREE:
>  		advise(_("The <src> part of the refspec is a tree object.\n"
>  			 "Did you mean to tag a new tree by pushing to\n"
>  			 "'%s:refs/tags/%s'?"),
>  		       matched_src_name, dst_value);
> -	} else if (type == OBJ_BLOB) {
> +		break;
> +	case OBJ_BLOB:
>  		advise(_("The <src> part of the refspec is a blob object.\n"
>  			 "Did you mean to tag a new blob by pushing to\n"
>  			 "'%s:refs/tags/%s'?"),
>  		       matched_src_name, dst_value);
> -	} else {
> +		break;
> +	default:
>  		BUG("'%s' should be commit/tag/tree/blob, is '%d'",
>  		    matched_src_name, type);

We can't remove the `type` variable in this patch already -- it's still
used by this call to `BUG()`. But we can drop the variable in the next
patch, where that call is converted to `advise()`.

So I'd recommend to either move this patch to after the next patch or to
keep the `type` variable here and remove it in the next patch.

Patrick