Re: [PATCH 03/10] xdiff: don't waste time guessing the number of lines
- From
Ezekiel Newren <ezekielnewren@gmail.com>
- Date
- Jan 21, 2026, 21:12 UTC
- Message-ID
- <CAH=ZcbCbz6MB9-9Ehskk2+27GMXXewmAzRcGyN_bBi8s5Ksxjg@mail.gmail.com>
- In-Reply-To
- <208da094-8a5d-4f16-b42b-5d5204576b5f@gmail.com>
On Tue, Jan 20, 2026 at 8:02 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
Show 18 quoted lines
>
> 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. 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. 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."