Re: [PATCH 03/10] xdiff: don't waste time guessing the number of lines

[Phillip Wood <phillip.wood123@gmail.com>]

On 21/01/2026 21:12, Ezekiel Newren wrote:
> On Tue, Jan 20, 2026 at 8:02 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> On 02/01/2026 18:52, Ezekiel Newren via GitGitGadget wrote:
>>> From: Ezekiel Newren <ezekielnewren@gmail.com>
>>>
>>> All lines must be read anyway, so classify them after they're read in.
>>> Also move the memset() into xdl_init_classifier().
>>
>> So instead of looping over the input lines one and a bit times (the bit
>> being from xdl_guess_lines) we now loop over them twice as we split them
>> first and then classify them in a separate loop. It does save some work
>> not to call xdl_guess_lines but it is unclear if that offsets
>> classifying them in a separate loop.
>>
>>> +     for (size_t i = 0; i < xe->xdf1.nrec; i++) {
>>> +             xrecord_t *rec = &xe->xdf1.recs[i];
>>> +             xdl_classify_record(1, &cf, rec);
>>
>> We seem to have lost the error handling if xdl_classify_record() fails.
> 
> The error handling was not "lost" it was deliberately removed. 

That's the sort of thing that needs to be explained in the commit message.

> The
> only way in which xdl_classify_record() could fail is by a failed
> memory allocation. On the Rust side this would result in a panic
> (panic means something different in Rust vs C) in which case C could
> not possibly recover.

There is no rust code in xdiff at the moment so we don't panic on 
failure. In git we'll die() because xdl_malloc() and friends are defined 
as xmalloc() etc. which die on allocation failure. However anyone else 
picking up this code and using a different allocator that does not die 
on allocation failure will expect the error to be propagated.

If you want to stop supporting other allocators then you should propose 
a patch to do so, not silently slip the change into this patch.

Thanks

Phillip

> Also for operations like Vec.push() in Rust it's
> assumed that memory management functions will never fail and if they
> do they crash the program with no chance of recovery (unless you
> account for panic unwinding which is really ugly). It seems a lot of
> arguments about ivec and my xdiff cleanups are "We don't do things
> this way in Git/C" I'm aware of many of these arguments and I'm trying
> to address them with a more specific answer of "Yes, but that's not
> how things are done in Rust and all of this is to prepare the code for
> conversion to Rust and some things shouldn't, or even, cannot be done
> the C way in Rust."