Re: [PATCH 3/4] add: introduce '--resolved' option

Michael Montalbo <[email protected]> writes:

> On Wed, Jul 29, 2026 at 8:17 AM Junio C Hamano <[email protected]> wrote:
>>
>>
>> Left unchecked, this loop may end up scanning a large binary file to
>> the end in vain.  We may squeeze in something like this to punt
>> early.
>>
>>  merge-ll.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/merge-ll.c b/merge-ll.c
>> index 5e5044b9e3..ef5287dee8 100644
>> --- a/merge-ll.c
>> +++ b/merge-ll.c
>> @@ -516,6 +516,9 @@ int has_conflict_markers(struct index_state *istate, const char *path)
>>                         has_markers = 1;
>>                         break;
>>                 }
>> +               if (buffer_is_binary(sb.buf,
>> +                                    ULONG_MAX <= sb.len ? ULONG_MAX : sb.len))
>> +                       break;
>>         }
>
> Should this check be before the conflict marker line check in case the first
> iteration accidentally matches for a binary file and breaks with
> has_markers = 1?

If we misidentify early, that is a desirable outcome, isn't it?

We did not have to scan much and we gave control back to the user as
soon as we saw "<<<<<<", telling them that we refused to add the path
to the index, so that the user can inspect the situation more
deeply.  In this application, false positives are much better than
false negatives, and failing early is better than failing late.

So, no, I do not think so, even though in practice I do not think it
would matter either way.

A question that may have much more impact is whether the attribute
system should have any say in this code path.  I am somewhat torn on
this.