Re: [PATCH v2] merge-tree: fix segmentation fault in read-only repositories
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Sep 22, 2022, 19:52 UTC
- Message-ID
- <p7896r78-o578-80nr-s3n5-sr01o9q8prs9@tzk.qr>
- In-Reply-To
- <xmqqfsgk1i2t.fsf@gitster.g>
Hi Junio,
On Wed, 21 Sep 2022, Junio C Hamano wrote:
Show 27 quoted lines
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > + const struct object_id *tree_oid;
> >
> > parent1 = get_merge_parent(branch1);
> > if (!parent1)
> > @@ -446,7 +447,8 @@ static int real_merge(struct merge_tree_options *o,
> > if (o->show_messages == -1)
> > o->show_messages = !result.clean;
> >
> > - printf("%s%c", oid_to_hex(&result.tree->object.oid), line_termination);
> > + tree_oid = result.tree ? &result.tree->object.oid : null_oid();
> > + printf("%s%c", oid_to_hex(tree_oid), line_termination);
>
> As we are not using tree_oid anywhere else, it might be cleaner to
> switch on result.tree more explicitly, especially given that the
> return statement also uses result.tree to decide what exit status to
> use, e.g.
>
> if (result.tree)
> fputs(oid_to_hex(&result.tree->object.oid), stdout);
> else
> fputs(oid_to_hex(null_oid()), stdout);
> fputc(line_termination, stdout);
>
> but that is merely "might be cleaner".I would call it "more readable", even, not just "cleaner" :-)
Show 9 quoted lines
> > - return !result.clean; /* result.clean < 0 handled above */ > > + return !result.tree || !result.clean; /* result.clean < 0 handled above */ > > The original returned 1 when we failed to cleanly merge and 0 when > we cleanly merged. Now, if we failed to come up with a merged tree, > result.tree would be NULL and we return 1 from the function, because > we failed to cleanly merge. > > OK. These negations make my head spin X-<.
And here I thought that you couldn't possibly not like if I didn't refrain from not avoiding double negations...
Ciao, Dscho