Re: [PATCH v5 13/13] xdiff: change type of xdfile_t.changed from char to bool
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Sep 24, 2025, 10:21 UTC
- Message-ID
- <a17e00fa-bf4b-4b53-817b-297236178101@gmail.com>
- In-Reply-To
- <975e845bfa80fe838433f7f64759a72f6d565aea.1758662670.git.gitgitgadget@gmail.com>
On 23/09/2025 22:24, Ezekiel Newren via GitGitGadget wrote:
Show 11 quoted lines
> From: Ezekiel Newren <ezekielnewren@gmail.com> > > The only values possible for 'changed' is 1 and 0, which exactly maps > to a bool type. It might not look like this is the case because > matches1 and matches2 (which use to be dis1, and dis2) were also char > and were assigned numerical values within a few lines of 'changed' > (what used to be rchg). > > Using NONE, SOME, TOO_MANY for matches1[i]/matches2[j], and true/false > for changed[k] makes it clear to future readers that these are > logically separate concepts.
Nicely explained - I think this change is a very good idea and separating it out like this makes it much clearer what's going on compared to V4.
Thanks
Phillip
Show 167 quoted lines
> Best-viewed-with: --color-words
> Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
> ---
> xdiff/xdiffi.c | 12 ++++++------
> xdiff/xhistogram.c | 8 ++++----
> xdiff/xpatience.c | 8 ++++----
> xdiff/xprepare.c | 12 ++++++------
> xdiff/xtypes.h | 2 +-
> 5 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
> index 5535452061..b902be9d0e 100644
> --- a/xdiff/xdiffi.c
> +++ b/xdiff/xdiffi.c
> @@ -278,10 +278,10 @@ int xdl_recs_cmp(xdfile_t *xdf1, long off1, long lim1,
> */
> if (off1 == lim1) {
> for (; off2 < lim2; off2++)
> - xdf2->changed[xdf2->rindex[off2]] = 1;
> + xdf2->changed[xdf2->rindex[off2]] = true;
> } else if (off2 == lim2) {
> for (; off1 < lim1; off1++)
> - xdf1->changed[xdf1->rindex[off1]] = 1;
> + xdf1->changed[xdf1->rindex[off1]] = true;
> } else {
> xdpsplit_t spl;
> spl.i1 = spl.i2 = 0;
> @@ -753,8 +753,8 @@ static int group_slide_down(xdfile_t *xdf, struct xdlgroup *g)
> {
> if (g->end < xdf->nrec &&
> recs_match(&xdf->recs[g->start], &xdf->recs[g->end])) {
> - xdf->changed[g->start++] = 0;
> - xdf->changed[g->end++] = 1;
> + xdf->changed[g->start++] = false;
> + xdf->changed[g->end++] = true;
>
> while (xdf->changed[g->end])
> g->end++;
> @@ -774,8 +774,8 @@ static int group_slide_up(xdfile_t *xdf, struct xdlgroup *g)
> {
> if (g->start > 0 &&
> recs_match(&xdf->recs[g->start - 1], &xdf->recs[g->end - 1])) {
> - xdf->changed[--g->start] = 1;
> - xdf->changed[--g->end] = 0;
> + xdf->changed[--g->start] = true;
> + xdf->changed[--g->end] = false;
>
> while (xdf->changed[g->start - 1])
> g->start--;
> diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c
> index 15ca15f6b0..6dc450b1fe 100644
> --- a/xdiff/xhistogram.c
> +++ b/xdiff/xhistogram.c
> @@ -318,11 +318,11 @@ redo:
>
> if (!count1) {
> while(count2--)
> - env->xdf2.changed[line2++ - 1] = 1;
> + env->xdf2.changed[line2++ - 1] = true;
> return 0;
> } else if (!count2) {
> while(count1--)
> - env->xdf1.changed[line1++ - 1] = 1;
> + env->xdf1.changed[line1++ - 1] = true;
> return 0;
> }
>
> @@ -335,9 +335,9 @@ redo:
> else {
> if (lcs.begin1 == 0 && lcs.begin2 == 0) {
> while (count1--)
> - env->xdf1.changed[line1++ - 1] = 1;
> + env->xdf1.changed[line1++ - 1] = true;
> while (count2--)
> - env->xdf2.changed[line2++ - 1] = 1;
> + env->xdf2.changed[line2++ - 1] = true;
> result = 0;
> } else {
> result = histogram_diff(xpp, env,
> diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c
> index 14092ffb86..669b653580 100644
> --- a/xdiff/xpatience.c
> +++ b/xdiff/xpatience.c
> @@ -331,11 +331,11 @@ static int patience_diff(xpparam_t const *xpp, xdfenv_t *env,
> /* trivial case: one side is empty */
> if (!count1) {
> while(count2--)
> - env->xdf2.changed[line2++ - 1] = 1;
> + env->xdf2.changed[line2++ - 1] = true;
> return 0;
> } else if (!count2) {
> while(count1--)
> - env->xdf1.changed[line1++ - 1] = 1;
> + env->xdf1.changed[line1++ - 1] = true;
> return 0;
> }
>
> @@ -347,9 +347,9 @@ static int patience_diff(xpparam_t const *xpp, xdfenv_t *env,
> /* are there any matching lines at all? */
> if (!map.has_matches) {
> while(count1--)
> - env->xdf1.changed[line1++ - 1] = 1;
> + env->xdf1.changed[line1++ - 1] = true;
> while(count2--)
> - env->xdf2.changed[line2++ - 1] = 1;
> + env->xdf2.changed[line2++ - 1] = true;
> xdl_free(map.entries);
> return 0;
> }
> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
> index e1d575f779..070d220f3b 100644
> --- a/xdiff/xprepare.c
> +++ b/xdiff/xprepare.c
> @@ -273,7 +273,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
>
> /*
> * Create temporary arrays that will help us decide if
> - * changed[i] should remain 0 or become 1.
> + * changed[i] should remain false, or become true.
> */
> if (!XDL_CALLOC_ARRAY(matches1, xdf1->nrec + 1)) {
> status = -1;
> @@ -305,16 +305,16 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
>
> /*
> * Use temporary arrays to decide if changed[i] should remain
> - * 0 or become 1.
> + * false, or become true.
> */
> for (nreff = 0, i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart];
> i <= xdf1->dend; i++, recs++) {
> if (matches1[i] == SOME ||
> (matches1[i] == TOO_MANY && !xdl_clean_mmatch(matches1, i, xdf1->dstart, xdf1->dend))) {
> xdf1->rindex[nreff++] = i;
> - /* changed[i] remains 0 */
> + /* changed[i] remains false */
> } else
> - xdf1->changed[i] = 1;
> + xdf1->changed[i] = true;
> }
> xdf1->nreff = nreff;
>
> @@ -323,9 +323,9 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
> if (matches2[i] == SOME ||
> (matches2[i] == TOO_MANY && !xdl_clean_mmatch(matches2, i, xdf2->dstart, xdf2->dend))) {
> xdf2->rindex[nreff++] = i;
> - /* changed[i] remains 0 */
> + /* changed[i] remains false */
> } else
> - xdf2->changed[i] = 1;
> + xdf2->changed[i] = true;
> }
> xdf2->nreff = nreff;
>
> diff --git a/xdiff/xtypes.h b/xdiff/xtypes.h
> index c4b5d2d8fa..f145abba3e 100644
> --- a/xdiff/xtypes.h
> +++ b/xdiff/xtypes.h
> @@ -48,7 +48,7 @@ typedef struct s_xdfile {
> xrecord_t *recs;
> long nrec;
> long dstart, dend;
> - char *changed;
> + bool *changed;
> long *rindex;
> long nreff;
> } xdfile_t;