Re: [PATCH 0/9] Xdiff cleanup part2
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Oct 21, 2025, 13:28 UTC
- Message-ID
- <93ec3dbf-ad98-4038-84e9-9ca12b7481a0@gmail.com>
- In-Reply-To
- <pull.2070.git.git.1760563101.gitgitgadget@gmail.com>
Hi Ezekiel
On 15/10/2025 22:18, Ezekiel Newren via GitGitGadget wrote:
Show 7 quoted lines
> Maintainer note: This patch series builds on top of en/xdiff-cleanup and > am/xdiff-hash-tweak (both of which are now in master). > > The primary goal of this patch series is to convert every field's type in > xrecord_t and xdfile_t to be unambiguous, in preparation to make it more > Rust FFI friendly. Additionally the ha field in xrecord_t is split into > line_hash and minimal_perfect hash.
Given that this series changes the types of all the "long" struct members to "size_t" I was surprised to see that it adds so many "(long)" casts. At the end of this series there are 38 lines in xdiff/ that contain "(long)" compared to just 4 in master. I had expected that as we'd converted all the members to "size_t" there would be no need to keep using "long" in the code. As rust is going to be optional for quite a while I think we should clean up the C code to avoid casting between "long" and "size_t"
Thanks
Phillip
Show 68 quoted lines
> The order of some of the fields has changed as called out by the commit
> messages.
>
> Before:
>
> typedef struct s_xrecord {
> char const *ptr;
> long size;
> unsigned long ha;
> } xrecord_t;
>
> typedef struct s_xdfile {
> xrecord_t *recs;
> long nrec;
> long dstart, dend;
> bool *changed;
> long *rindex;
> long nreff;
> } xdfile_t;
>
>
> After part 2
>
> typedef struct s_xrecord {
> uint8_t const *ptr;
> size_t size;
> uint64_t line_hash;
> size_t minimal_perfect_hash;
> } xrecord_t;
>
> typedef struct s_xdfile {
> xrecord_t *recs;
> size_t nrec;
> bool *changed;
> size_t *reference_index;
> size_t nreff;
> ssize_t dstart, dend;
> } xdfile_t;
>
>
> Ezekiel Newren (9):
> xdiff: use ssize_t for dstart/dend, make them last in xdfile_t
> xdiff: make xrecord_t.ptr a uint8_t instead of char
> xdiff: use size_t for xrecord_t.size
> xdiff: use unambiguous types in xdl_hash_record()
> xdiff: split xrecord_t.ha into line_hash and minimal_perfect_hash
> xdiff: make xdfile_t.nrec a size_t instead of long
> xdiff: make xdfile_t.nreff a size_t instead of long
> xdiff: change rindex from long to size_t in xdfile_t
> xdiff: rename rindex -> reference_index
>
> xdiff-interface.c | 2 +-
> xdiff/xdiffi.c | 29 +++++++++++------------
> xdiff/xemit.c | 28 +++++++++++-----------
> xdiff/xhistogram.c | 4 ++--
> xdiff/xmerge.c | 30 ++++++++++++------------
> xdiff/xpatience.c | 14 +++++------
> xdiff/xprepare.c | 58 +++++++++++++++++++++++-----------------------
> xdiff/xtypes.h | 15 ++++++------
> xdiff/xutils.c | 32 ++++++++++++-------------
> xdiff/xutils.h | 6 ++---
> 10 files changed, 109 insertions(+), 109 deletions(-)
>
>
> base-commit: 143f58ef7535f8f8a80d810768a18bdf3807de26
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2070%2Fezekielnewren%2Fxdiff_cleanup_part2-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2070/ezekielnewren/xdiff_cleanup_part2-v1
> Pull-Request: https://github.com/git/git/pull/2070