From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 20 Sep 2025 17:16:21 GMT
Subject: Re: [PATCH v3 01/10] xdiff: delete static forward declarations in xprepare
Message-ID: <xmqqh5wxxcwq.fsf@gitster.g>
In-Reply-To: <784cffcef564b31a32c401b35f33610b85126f7b.1758294992.git.gitgitgadget@gmail.com>

"Ezekiel Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Ezekiel Newren <ezekielnewren@gmail.com>
>
> Move xdl_prepare_env() later in the file to avoid the need
> for static forward declarations.
>
> Best-viewed-with: --color-moved

Two comments.

 - This is a bit unusual to see in the trailer.

 - It turned out that it was a very effective way to spot a typo for
   me.  You should try it yourself before you send out your patches
   ;-).

> -int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
> -		    xdfenv_t *xe) {
> -	long enl1, enl2, sample;
> -	xdlclassifier_t cf;
> -
> -	memset(&cf, 0, sizeof(cf));
> ...
> -		xdl_free_ctx(&xe->xdf1);
> -		xdl_free_classifier(&cf);
> -		return -1;
> -	}

The "--color-moved" painted the line above, with a single closing
brace, as removed, which stood out.  It turns out that ...

> @@ -460,3 +394,53 @@ static int xdl_optimize_ctxs(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdf2
>  
>  	return 0;
>  }
> +
> +int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
> +		    xdfenv_t *xe) {
> +	long enl1, enl2, sample;
> +	xdlclassifier_t cf;
> +
> +	memset(&cf, 0, sizeof(cf));
> ...
> +		xdl_free_ctx(&xe->xdf2);
> +		xdl_free_ctx(&xe->xdf1);
> +		xdl_free_classifier(&cf);
> +		return -1;
> +	    }

... the corresponding line in the postimage was shown as newly
added.  That was because it was indented incorrectly.

> +	xdl_free_classifier(&cf);
> +
> +	return 0;
> +}

If I do not spot any other issues in the series, I may just "rebase
-i" to correct this single line to reduce the risk of mistakes,
instead of asking you to send an update.  We'll see.

The change is sensible, and the proposed log message does a good
job, too.

Thanks.