From: SZEDER Gábor Date: Thu, 26 Mar 2026 06:26:11 GMT Subject: Re: [PATCH v2 0/5] Xdiff cleanup part 3 Message-ID: In-Reply-To: On Wed, Mar 25, 2026 at 09:11:00PM +0000, Ezekiel Newren via GitGitGadget wrote: > v2 is a radical departure from v1 Changes in v2: > > * make the flow of xdl_cleanup_records() easier to follow > > There is no performance or behavioral change introduced in this patch > series. > > === original cover letter bellow === > > Patch series summary: > > * patch 1: Introduce the ivec type > * patch 2: Create the function xdl_do_classic_diff() > * patches 3-4: generic cleanup > * patches 5-8: convert from dstart/dend (in xdfile_t) to > delta_start/delta_end (in xdfenv_t) > * patches 9-10: move xdl_cleanup_records(), and related, from xprepare.c to > xdiffi.c > > Things that will be addressed in future patch series: > > * Make xdl_cleanup_records() easier to read > * convert recs/nrec into an ivec > * convert changed to an ivec > * remove reference_index/nreff from xdfile_t and turn it into an ivec > * splitting minimal_perfect_hash out as its own ivec > * improve the performance of the classifier and parsing/hashing lines > > === before this patch series typedef struct s_xdfile { xrecord_t *recs; > size_t nrec; ptrdiff_t dstart, dend; bool *changed; size_t *reference_index; > size_t nreff; } xdfile_t; > > typedef struct s_xdfenv { xdfile_t xdf1, xdf2; } xdfenv_t; > > === after this patch series typedef struct s_xdfile { xrecord_t *recs; > size_t nrec; bool *changed; size_t *reference_index; size_t nreff; } > xdfile_t; > > typedef struct s_xdfenv { xdfile_t xdf1, xdf2; size_t delta_start, > delta_end; size_t mph_size; } xdfenv_t; Please make sure that each commit in this series can be built with DEVELOPER=1, which enables a bunch of additional compiler warnings. While the last commit can be built with all those warnings, the three in the middle fail with sign comparison errors. > Ezekiel Newren (5): > xdiff/xdl_cleanup_records: delete local recs pointer > xdiff/xdl_cleanup_records: make limits more clear CC xdiff/xprepare.o xdiff/xprepare.c: In function ‘xdl_cleanup_records’: xdiff/xprepare.c:307:54: error: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare] 307 | action1[i] = (nm == 0) ? DISCARD: nm >= mlim1 ? INVESTIGATE: KEEP; | ^~ xdiff/xprepare.c:314:54: error: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare] 314 | action2[i] = (nm == 0) ? DISCARD: nm >= mlim2 ? INVESTIGATE: KEEP; | ^~ cc1: all warnings being treated as errors make: *** [Makefile:2923: xdiff/xprepare.o] Error 1 > xdiff/xdl_cleanup_records: make setting action easier to follow CC xdiff/xprepare.o xdiff/xprepare.c: In function ‘xdl_cleanup_records’: xdiff/xprepare.c:309:29: error: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare] 309 | else if (nm < mlim1) | ^ xdiff/xprepare.c:321:29: error: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare] 321 | else if (nm < mlim2) | ^ cc1: all warnings being treated as errors make: *** [Makefile:2923: xdiff/xprepare.o] Error 1 > xdiff/xdl_cleanup_records: simplify INVESTIGATE handling for clarity Same error as the last one. > xdiff/xdl_cleanup_records: use unambiguous types Good. > > xdiff/xprepare.c | 89 ++++++++++++++++++++++++++++++++---------------- > 1 file changed, 59 insertions(+), 30 deletions(-)