Junio C Hamano <gitster@xxxxxxxxx> writes:
Sorry, this will be the last message from me on this topic for now.
> we'd probably need to factor out the condition used in the above
> into a helper function, e.g.
>
> static int cannot_be_the_same(struct diff_filespec *one, struct diff_filespec *two)
The naming of this helper is tricky. In both potential callers,
what we want to see is "one and two may be different, we cannot say
they are the same with certainty", so "cannot be the same" is a
misnomer. Worse, the negated form is hard to grok.
Perhaps "may_differ()" is a more correct name. If either side is
NULL oid, we cannot say they are the same, so it is true. If two
oid that are not NULL oid are the same, there is no possibility that
they are different, so we return false. And two oid that are not
NULL oid are different, we know they are different, so we return
true.
> {
> if ((oideq(&one->oid, &null_oid) || oideq(&two->oid, &null_oid))
> return 1;
> else if (oideq(&one->oid, &two->oid))
> return 0;
> else
> return 1;
> }
>
> and rewrite the conditional in fill_metainfo() to
>
> if (one && two && cannot_be_the_same(one, two)) {
> ...
And this becomes much easier to read and understand, i.e.
if (one && two && may_differ(one, two)) {
... create the index one->oid..two->oid header ...
> The "second best fix" could then become a single liner:
>
> same_contents = !cannot_be_the_same(one, two);
>
> using the helper.
And this becomes
same_contents = !may_differ(one, two);
meaning that "there is no possibility that one and two are
different". That allows us to optimize out the invocation of the
xdl machinery.
