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

[Elijah Newren <newren@gmail.com>]

On Sun, Feb 16, 2025 at 8:37 AM Phillip Wood via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> 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.

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