Patchwork D2586: cext: refactor cleanup code in bdiff()

login
register
mail settings
Submitter phabricator
Date March 3, 2018, 4:28 p.m.
Message ID <differential-rev-PHID-DREV-v37ohdp45iubrktlqtcy-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/28787/
State Superseded
Headers show

Comments

phabricator - March 3, 2018, 4:28 p.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  A future commit will need to introduce additional cleanup code.
  
  We refactor the cleanup code to check NULL before calling free().
  We also initialize these variables as NULL.
  
  We set the out of memory exception explicitly, so we can just return
  result.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2586

AFFECTED FILES
  mercurial/cext/bdiff.c

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - March 3, 2018, 8:16 p.m.
yuja added a comment.


  Queued, thanks.

INLINE COMMENTS

> bdiff.c:148
> +		bdiff_freehunks(l.next);
> +	}
> +	return result;

Nit: these "if"s aren't necessary

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2586

To: indygreg, #hg-reviewers, yuja
Cc: mercurial-devel

Patch

diff --git a/mercurial/cext/bdiff.c b/mercurial/cext/bdiff.c
--- a/mercurial/cext/bdiff.c
+++ b/mercurial/cext/bdiff.c
@@ -62,11 +62,11 @@ 
 {
 	char *sa, *sb, *rb, *ia, *ib;
 	PyObject *result = NULL;
-	struct bdiff_line *al, *bl;
+	struct bdiff_line *al = NULL, *bl = NULL;
 	struct bdiff_hunk l, *h;
 	int an, bn, count;
 	Py_ssize_t len = 0, la, lb, li = 0, lcommon = 0, lmax;
-	PyThreadState *_save;
+	PyThreadState *_save = NULL;
 
 	l.next = NULL;
 
@@ -89,12 +89,16 @@ 
 
 	an = bdiff_splitlines(sa + lcommon, la - lcommon, &al);
 	bn = bdiff_splitlines(sb + lcommon, lb - lcommon, &bl);
-	if (!al || !bl)
-		goto nomem;
+	if (!al || !bl) {
+		PyErr_NoMemory();
+		goto cleanup;
+	}
 
 	count = bdiff_diff(al, an, bl, bn, &l);
-	if (count < 0)
-		goto nomem;
+	if (count < 0) {
+		PyErr_NoMemory();
+		goto cleanup;
+	}
 
 	/* calculate length of output */
 	la = lb = 0;
@@ -110,7 +114,7 @@ 
 	result = PyBytes_FromStringAndSize(NULL, len);
 
 	if (!result)
-		goto nomem;
+		goto cleanup;
 
 	/* build binary patch */
 	rb = PyBytes_AsString(result);
@@ -130,13 +134,19 @@ 
 		lb = h->b2;
 	}
 
-nomem:
+cleanup:
 	if (_save)
 		PyEval_RestoreThread(_save);
-	free(al);
-	free(bl);
-	bdiff_freehunks(l.next);
-	return result ? result : PyErr_NoMemory();
+	if (al) {
+		free(al);
+	}
+	if (bl) {
+		free(bl);
+	}
+	if (l.next) {
+		bdiff_freehunks(l.next);
+	}
+	return result;
 }
 
 /*