This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2586

cext: refactor cleanup code in bdiff()
ClosedPublic

Authored by indygreg on Mar 3 2018, 11:28 AM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHGb864f4536ca8: cext: refactor cleanup code in bdiff()
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.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Mar 3 2018, 11:28 AM
yuja accepted this revision.Mar 3 2018, 3:02 PM
This revision is now accepted and ready to land.Mar 3 2018, 3:02 PM
Closed by commit rHGb864f4536ca8: cext: refactor cleanup code in bdiff() (authored by indygreg). · Explain WhyMar 3 2018, 3:15 PM
This revision was automatically updated to reflect the committed changes.
yuja added a comment.Mar 3 2018, 3:16 PM

Queued, thanks.

mercurial/cext/bdiff.c
148

Nit: these "if"s aren't necessary

Revision Contents
Changeset List

PathPackages
Mmercurial/cext/bdiff.c (34 lines)

Diff 6465

mercurial/cext/bdiff.c

bdiff_freehunks(l.next); bdiff_freehunks(l.next);
return rl ? rl : PyErr_NoMemory(); return rl ? rl : PyErr_NoMemory();
} }
static PyObject *bdiff(PyObject *self, PyObject *args) static PyObject *bdiff(PyObject *self, PyObject *args)
{ {
char *sa, *sb, *rb, *ia, *ib; char *sa, *sb, *rb, *ia, *ib;
PyObject *result = NULL; PyObject *result = NULL;
struct bdiff_line *al, *bl; struct bdiff_line *al = NULL, *bl = NULL;
struct bdiff_hunk l, *h; struct bdiff_hunk l, *h;
int an, bn, count; int an, bn, count;
Py_ssize_t len = 0, la, lb, li = 0, lcommon = 0, lmax; Py_ssize_t len = 0, la, lb, li = 0, lcommon = 0, lmax;
PyThreadState *_save; PyThreadState *_save = NULL;
l.next = NULL; l.next = NULL;
if (!PyArg_ParseTuple(args, PY23("s#s#:bdiff", "y#y#:bdiff"), &sa, &la, if (!PyArg_ParseTuple(args, PY23("s#s#:bdiff", "y#y#:bdiff"), &sa, &la,
&sb, &lb)) &sb, &lb))
return NULL; return NULL;
if (la > UINT_MAX || lb > UINT_MAX) { if (la > UINT_MAX || lb > UINT_MAX) {
PyErr_SetString(PyExc_ValueError, "bdiff inputs too large"); PyErr_SetString(PyExc_ValueError, "bdiff inputs too large");
return NULL; return NULL;
} }
_save = PyEval_SaveThread(); _save = PyEval_SaveThread();
lmax = la > lb ? lb : la; lmax = la > lb ? lb : la;
for (ia = sa, ib = sb; li < lmax && *ia == *ib; ++li, ++ia, ++ib) for (ia = sa, ib = sb; li < lmax && *ia == *ib; ++li, ++ia, ++ib)
if (*ia == '\n') if (*ia == '\n')
lcommon = li + 1; lcommon = li + 1;
/* we can almost add: if (li == lmax) lcommon = li; */ /* we can almost add: if (li == lmax) lcommon = li; */
an = bdiff_splitlines(sa + lcommon, la - lcommon, &al); an = bdiff_splitlines(sa + lcommon, la - lcommon, &al);
bn = bdiff_splitlines(sb + lcommon, lb - lcommon, &bl); bn = bdiff_splitlines(sb + lcommon, lb - lcommon, &bl);
if (!al || !bl) if (!al || !bl) {
goto nomem; PyErr_NoMemory();
goto cleanup;
}
count = bdiff_diff(al, an, bl, bn, &l); count = bdiff_diff(al, an, bl, bn, &l);
if (count < 0) if (count < 0) {
goto nomem; PyErr_NoMemory();
goto cleanup;
}
/* calculate length of output */ /* calculate length of output */
la = lb = 0; la = lb = 0;
for (h = l.next; h; h = h->next) { for (h = l.next; h; h = h->next) {
if (h->a1 != la || h->b1 != lb) if (h->a1 != la || h->b1 != lb)
len += 12 + bl[h->b1].l - bl[lb].l; len += 12 + bl[h->b1].l - bl[lb].l;
la = h->a2; la = h->a2;
lb = h->b2; lb = h->b2;
} }
PyEval_RestoreThread(_save); PyEval_RestoreThread(_save);
_save = NULL; _save = NULL;
result = PyBytes_FromStringAndSize(NULL, len); result = PyBytes_FromStringAndSize(NULL, len);
if (!result) if (!result)
goto nomem; goto cleanup;
/* build binary patch */ /* build binary patch */
rb = PyBytes_AsString(result); rb = PyBytes_AsString(result);
la = lb = 0; la = lb = 0;
for (h = l.next; h; h = h->next) { for (h = l.next; h; h = h->next) {
if (h->a1 != la || h->b1 != lb) { if (h->a1 != la || h->b1 != lb) {
len = bl[h->b1].l - bl[lb].l; len = bl[h->b1].l - bl[lb].l;
putbe32((uint32_t)(al[la].l + lcommon - al->l), rb); putbe32((uint32_t)(al[la].l + lcommon - al->l), rb);
putbe32((uint32_t)(al[h->a1].l + lcommon - al->l), putbe32((uint32_t)(al[h->a1].l + lcommon - al->l),
rb + 4); rb + 4);
putbe32((uint32_t)len, rb + 8); putbe32((uint32_t)len, rb + 8);
memcpy(rb + 12, bl[lb].l, len); memcpy(rb + 12, bl[lb].l, len);
rb += 12 + len; rb += 12 + len;
} }
la = h->a2; la = h->a2;
lb = h->b2; lb = h->b2;
} }
nomem: cleanup:
if (_save) if (_save)
PyEval_RestoreThread(_save); PyEval_RestoreThread(_save);
if (al) {
free(al); free(al);
}
if (bl) {
free(bl); free(bl);
}
if (l.next) {
bdiff_freehunks(l.next); bdiff_freehunks(l.next);
return result ? result : PyErr_NoMemory(); }
yujaUnsubmitted
Not Done

Nit: these "if"s aren't necessary

yuja: Nit: these "if"s aren't necessary
return result;
} }
/* /*
* If allws != 0, remove all whitespace (' ', \t and \r). Otherwise, * If allws != 0, remove all whitespace (' ', \t and \r). Otherwise,
* reduce whitespace sequences to a single space and trim remaining whitespace * reduce whitespace sequences to a single space and trim remaining whitespace
* from end of lines. * from end of lines.
*/ */
static PyObject *fixws(PyObject *self, PyObject *args) static PyObject *fixws(PyObject *self, PyObject *args)