Re: [PATCH] branch: trivial style fix

From: Jeff King <peff@peff.net>
Date: Oct 5, 2018, 16:21 UTC
Message-ID: <20181005162152.GA11254@sigill.intra.peff.net>
In-Reply-To: <20181005095213.12509-1-taoqy@ls-a.me>

On Fri, Oct 05, 2018 at 05:52:14PM +0800, Tao Qingyun wrote:

Show 17 quoted lines

> diff --git a/builtin/branch.c b/builtin/branch.c
> index c396c41533..52aad0f602 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -222,10 +222,11 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
>  		if (!head_rev)
>  			die(_("Couldn't look up commit object for HEAD"));
>  	}
> -	for (i = 0; i < argc; i++, strbuf_reset(&bname)) {
> +	for (i = 0; i < argc; i++) {
>  		char *target = NULL;
>  		int flags = 0;
>  
> +		strbuf_reset(&bname);
>  		strbuf_branchname(&bname, argv[i], allowed_interpret);
>  		free(name);
>  		name = mkpathdup(fmt, bname.buf);

This one isn't just style: it switches the reset from the end of the loop to the beginning of the loop. So we'd need to make sure that we are not depending on its contents going into the first iteration, nor coming out of the last one.

Looking at the surrounding code, I think it is OK.

Show 9 quoted lines

> @@ -716,8 +717,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>  		print_columns(&output, colopts, NULL);
>  		string_list_clear(&output, 0);
>  		return 0;
> -	}
> -	else if (edit_description) {
> +	} else if (edit_description) {
>  		const char *branch_name;
>  		struct strbuf branch_ref = STRBUF_INIT;

And this one is obviously correct.

-Peff