Re: [PATCH v3 04/10] xdiff: delete xdl_get_rec() in xemit

From: Ezekiel Newren <ezekielnewren@gmail.com>
Date: Sep 21, 2025, 15:07 UTC
Message-ID: <CAH=ZcbD6aw-=93URRjYKfYvt+0z2YrcwfyY9TmZHSsXXsXiB+g@mail.gmail.com>
In-Reply-To: <cd39a321-a6c4-4126-a716-dc0b0cffa68d@gmail.com>

On Sun, Sep 21, 2025 at 7:06 AM Phillip Wood <phillip.wood123@gmail.com> 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/