Re: [PATCH 2/5] merge-tree: remove redundant code

From: Elijah Newren <newren@gmail.com>
Date: Feb 17, 2025, 20:15 UTC
Message-ID: <CABPp-BGTSciJMRWBGe9qOFh5wGuLppB6L+v9J5-KVdbNc6H3Hw@mail.gmail.com>
In-Reply-To: <16fec87766f97d46a337f5c514f1aec0668546ec.1739723830.git.gitgitgadget@gmail.com>

On Sun, Feb 16, 2025 at 8:37 AM Phillip Wood via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 6 quoted lines

>
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> real_merge() only ever returns "0" or "1" as it dies if the merge status
> is less than zero. Therefore the check for "result < 0" is redundant and
> the result variable is not needed.

Indeed, the only return statement in real_merge(), occurring on the
last line of the function, is even:
    return !result.clean; /* result.clean < 0 handled above */

However, it might be worth adding to the commit message some comments about o->use_stdin here. When o->use_stdin is true, that the program exit status is 0 for both successful merges and conflicts but the conflict status for each individual commit is printed as part of the output. As such, the return status isn't used in those cases and real_merge() might as well be a void function. However, when o->use_stdin is false, the exit status from real_merge is used, which is why that callsite (not visibile in this patch since it is unmodified) still pays attention to real_merge()'s return status.

Show 38 quoted lines

> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  builtin/merge-tree.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
> index 57f4340faba..3c73482f2b0 100644
> --- a/builtin/merge-tree.c
> +++ b/builtin/merge-tree.c
> @@ -601,7 +601,6 @@ int cmd_merge_tree(int argc,
>                 line_termination = '\0';
>                 while (strbuf_getline_lf(&buf, stdin) != EOF) {
>                         struct strbuf **split;
> -                       int result;
>                         const char *input_merge_base = NULL;
>
>                         split = strbuf_split(&buf, ' ');
> @@ -618,16 +617,14 @@ int cmd_merge_tree(int argc,
>                         if (input_merge_base && split[2] && split[3] && !split[4]) {
>                                 strbuf_rtrim(split[2]);
>                                 strbuf_rtrim(split[3]);
> -                               result = real_merge(&o, input_merge_base, split[2]->buf, split[3]->buf, prefix);
> +                               real_merge(&o, input_merge_base, split[2]->buf, split[3]->buf, prefix);
>                         } else if (!input_merge_base && !split[2]) {
> -                               result = real_merge(&o, NULL, split[0]->buf, split[1]->buf, prefix);
> +                               real_merge(&o, NULL, split[0]->buf, split[1]->buf, prefix);
>                         } else {
>                                 die(_("malformed input line: '%s'."), buf.buf);
>                         }
>                         maybe_flush_or_die(stdout, "stdout");
>
> -                       if (result < 0)
> -                               die(_("merging cannot continue; got unclean result of %d"), result);
>                         strbuf_list_free(split);
>                 }
>                 strbuf_release(&buf);
> --
> gitgitgadget

Looks good.