Re: [PATCH v2 0/5] Xdiff cleanup part 3

["SZEDER Gábor" <szeder.dev@gmail.com>]

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(-)