Re: [PATCH v5] branch: support for shortcuts like @{-1}, completed

From

Rubén Justo <rjusto@gmail.com>

Date

Oct 9, 2022, 21:26 UTC

Message-ID

<dacb1f40-2765-5978-1131-f78ba919e6e6@gmail.com>

In-Reply-To

<xmqqa6647qx2.fsf@gitster.g>

On 9/10/22 21:11, Junio C Hamano wrote:

> Rubén Justo <rjusto@gmail.com> writes:
> 
>  * cmd_foo() should not return an negative value.

Yes, the refactoring we already discussed early in this thread.

> 
>  * branch_name used in the calls to error() could point at buf.buf
>    that holds the expansion of @{-1}, but buf was released way too
>    early, leading to a use-after-free.

> 
>  * Style: if/else if/else cascade whose one arm has multiple
>    statements and requires {braces} around it should have {braces}
>    around all of its arms.

Ok.

> 
>  * each arm in the top-level if/else if/else cascade for "git
>    branch" subcommands were more or less independent, and there
>    wasn't anything common that they need to execute after exiting
>    the cascade.  Unconditionally returning from the arm for the
>    edit-description subcommand seems to make the logic flow easier
>    to read.

Mmm, I don't feel the same here, we already discussed about this. Maybe?:

diff --git a/builtin/branch.c b/builtin/branch.c
index 17853225fa..307073cc47 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -817,7 +817,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
                strbuf_release(&branch_ref);
                strbuf_release(&buf);
 
-               return ret;
+               if (ret)
+                       return ret; /* some failure happened */
        } else if (copy) {
                if (!argc)
                        die(_("branch name required"));


not much important, though.

You can squash the changes in the commit or if you need me to send a v6,
please let me know. Thank you for your careful review.