From: Ezekiel Newren Date: Sun, 21 Sep 2025 15:07:27 GMT Subject: Re: [PATCH v3 04/10] xdiff: delete xdl_get_rec() in xemit Message-ID: In-Reply-To: On Sun, Sep 21, 2025 at 7:06 AM Phillip Wood wrote: > Patch 6 goes the other way and introduces a getter function that hides > the field accesses so I'm not sure why this one is so bad that it needs > to be removed. I've added a copy below of the two functions for easy reference. To quote myself[1]: ``` The fields rindex and ha of xdfile_t are specific to the classic diff (myers and minimal). I plan on creating a struct for classic diff, but there's a lot of cleanup that needs to be done before that can happen, and leaving ha in would make those cleanups harder to follow. ``` get_hash() is a scaffolding function that will reduce refactor churn. It changes a few times in this patch series alone, and will change a few more times before the code is cleaned up enough to delete it. By contrast, xdl_get_rec() merely performs an array index, which is so trivial that it doesn't justify having its own function. get_hash() reduces confusion because xdfile_t.ha is an array that is a sparse copy of xrecord_t.ha values from xdfile_t.recs. The field xrecord_t.ha is confusing on its own, as it is first used to store the hash of the line and later repurposed as a minimal perfect hash[2]. static long xdl_get_rec(xdfile_t *xdf, long ri, char const **rec) { *rec = xdf->recs[ri]->ptr; return xdf->recs[ri]->size; } static unsigned long get_hash(xdfile_t *xdf, long index) { return xdf->recs[xdf->rindex[index]]->ha; } [1] https://lore.kernel.org/git/0bacb1191dad2748d2afa79665f1293b0381bde1.1758294992.git.gitgitgadget@gmail.com/ [2] https://lore.kernel.org/git/af96763036e13480ed4e6dfedcade5b2c90e414c.1757274320.git.gitgitgadget@gmail.com/