Web lists-archives.com

Re: [PATCH] branch: trivial style fix




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

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

> @@ -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