[PATCH v2 0/4] xdiff: handle allocation failures

["Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com>]

Thanks to Junio for his comments on V1.

Changes since V1:

 * Patch 1 is new and addresses a memory leak noticed by Junio
 * Patch 2 is unchanged
 * Patch 3 now avoids a double free of xe1 on error
 * Patch 4 reworked handling of the return value

V1 Cover Letter: Other users of xdiff such as libgit2 need to be able to
handle allocation failures. This series fixes the few cases where we are not
doing this already.

Edward's patch[1] reminded me that I had these waiting to be submitted.

[1] https://lore.kernel.org/git/20220209013354.GB7@abe733c6e288/

Phillip Wood (4):
  xdiff: fix a memory leak
  xdiff: handle allocation failure in patience diff
  xdiff: refactor a function
  xdiff: handle allocation failure when merging

 xdiff/xdiffi.c     | 33 +++++++++++++++++----------------
 xdiff/xhistogram.c |  3 ---
 xdiff/xmerge.c     | 42 ++++++++++++++++++++++--------------------
 xdiff/xpatience.c  | 21 ++++++++++++---------
 4 files changed, 51 insertions(+), 48 deletions(-)


base-commit: 38062e73e009f27ea192d50481fcb5e7b0e9d6eb
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1140%2Fphillipwood%2Fwip%2Fxdiff-handle-allocation-failures-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1140/phillipwood/wip/xdiff-handle-allocation-failures-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1140

Range-diff vs v1:

 -:  ----------- > 1:  3fcb6c80367 xdiff: fix a memory leak
 1:  b8f88f1b9f8 = 2:  fec1f4a4152 xdiff: handle allocation failure in patience diff
 2:  8655bb0348d ! 3:  073a45e9e85 xdiff: refactor a function
     @@ Metadata
       ## Commit message ##
          xdiff: refactor a function
      
     -    Rather than having to remember exactly what to free after an
     -    allocation failure use the standard "goto out" pattern. This will
     -    simplify the next commit that starts handling currently unhandled
     -    failures.
     +    Use the standard "goto out" pattern rather than repeating very similar
     +    code after checking for each error. This will simplify the next commit
     +    that starts handling allocation failures that are currently ignored.
     +    On error xdl_do_diff() frees the environment so we need to take care
     +    to avoid a double free in that case. xdl_build_script() does not
     +    assign a result unless it is successful so there is no possibility of
     +    a double free if it fails.
      
          Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
      
     @@ xdiff/xmerge.c: static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,
       		xmparam_t const *xmp, mmbuffer_t *result)
       {
      -	xdchange_t *xscr1, *xscr2;
     --	xdfenv_t xe1, xe2;
     --	int status;
      +	xdchange_t *xscr1 = NULL, *xscr2 = NULL;
     -+	xdfenv_t xe1 = { 0 }, xe2 = { 0 };
     + 	xdfenv_t xe1, xe2;
     +-	int status;
      +	int status = -1;
       	xpparam_t const *xpp = &xmp->xpp;
       
     @@ xdiff/xmerge.c: static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,
       	result->size = 0;
       
      -	if (xdl_do_diff(orig, mf1, xpp, &xe1) < 0) {
     --		return -1;
     ++	if (xdl_do_diff(orig, mf1, xpp, &xe1) < 0)
     + 		return -1;
      -	}
      -	if (xdl_do_diff(orig, mf2, xpp, &xe2) < 0) {
      -		xdl_free_env(&xe1);
      -		return -1;
      -	}
     -+	if (xdl_do_diff(orig, mf1, xpp, &xe1) < 0)
     -+		goto out;
      +
      +	if (xdl_do_diff(orig, mf2, xpp, &xe2) < 0)
     -+		goto out;
     ++		goto free_xe1; /* avoid double free of xe2 */
      +
       	if (xdl_change_compact(&xe1.xdf1, &xe1.xdf2, xpp->flags) < 0 ||
       	    xdl_change_compact(&xe1.xdf2, &xe1.xdf1, xpp->flags) < 0 ||
     @@ xdiff/xmerge.c: int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2,
       	xdl_free_script(xscr1);
       	xdl_free_script(xscr2);
       
     +-	xdl_free_env(&xe1);
     + 	xdl_free_env(&xe2);
     ++ free_xe1:
     ++	xdl_free_env(&xe1);
     + 
     + 	return status;
     + }
 3:  3e935abba1d ! 4:  7e705d771b0 xdiff: handle allocation failure when merging
     @@ Commit message
      
       ## xdiff/xmerge.c ##
      @@ xdiff/xmerge.c: int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2,
     - 	status = 0;
     + 	    xdl_build_script(&xe2, &xscr2) < 0)
     + 		goto out;
     + 
     +-	status = 0;
       	if (!xscr1) {
       		result->ptr = xdl_malloc(mf2->size);
     -+		if (!result->ptr) {
     -+			status = -1;
     ++		if (!result->ptr)
      +			goto out;
     -+		}
     ++		status = 0;
       		memcpy(result->ptr, mf2->ptr, mf2->size);
       		result->size = mf2->size;
       	} else if (!xscr2) {
       		result->ptr = xdl_malloc(mf1->size);
     -+		if (!result->ptr) {
     -+			status = -1;
     ++		if (!result->ptr)
      +			goto out;
     -+		}
     ++		status = 0;
       		memcpy(result->ptr, mf1->ptr, mf1->size);
       		result->size = mf1->size;
       	} else {

-- 
gitgitgadget