From 0e98c30973c7060ca467ec5802eb6e52ff0a0304 Mon Sep 17 00:00:00 2001 From: krangelov Date: Sat, 16 Oct 2021 21:12:16 +0200 Subject: [PATCH] fix space leak in PySequence_AsHypos. PyList_FromHypos->PyTuple_FromHypos --- src/runtime/python/ffi.c | 55 +++++++------------ src/runtime/python/ffi.h | 2 +- src/runtime/python/pypgf.c | 2 +- src/runtime/python/tests/test_basic.py | 4 +- src/runtime/python/tests/test_transactions.py | 5 +- 5 files changed, 28 insertions(+), 40 deletions(-) diff --git a/src/runtime/python/ffi.c b/src/runtime/python/ffi.c index 147a03fd7..35888a99f 100644 --- a/src/runtime/python/ffi.c +++ b/src/runtime/python/ffi.c @@ -69,39 +69,34 @@ PySequence_AsHypos(PyObject *pyseq, Py_ssize_t *n_hypos) PgfTypeHypo *hypos = PyMem_RawMalloc(sizeof(PgfTypeHypo)*n); for (Py_ssize_t i = 0; i < n; i++) { - // PyObject *item = PySequence_GetItem(pyseq, i); - // if (!PyObject_TypeCheck(item, &pgf_HypoType)) { - // PyErr_SetString(PyExc_TypeError, "hypothesis must be of type Hypo"); - // return NULL; - // } - // HypoObject *hypo = (HypoObject *)item; - // hypos[i].bind_type = hypo->bind_type == Py_True ? 0 : 1; - // hypos[i].cid = PyUnicode_AsPgfText(hypo->cid); - // hypos[i].type = (PgfType) hypo->type; - PyObject *tup = PySequence_GetItem(pyseq, i); - if (!PyTuple_Check(tup)) { + if (!PyTuple_Check(tup) || PyTuple_Size(tup) != 3) { PyErr_SetString(PyExc_TypeError, "hypothesis must be a tuple"); + FreeHypos(hypos, i); return NULL; } - PyObject *t0 = PyTuple_GetItem(tup, 0); - if (!PyLong_Check(t0)) { + PyObject *t0 = PyTuple_GET_ITEM(tup, 0); + if (!PyBool_Check(t0)) { PyErr_SetString(PyExc_TypeError, "hypothesis bind type must be an boolean"); + FreeHypos(hypos, i); return NULL; } - hypos[i].bind_type = t0 == Py_True ? 0 : 1; + hypos[i].bind_type = t0 == Py_True ? PGF_BIND_TYPE_EXPLICIT : PGF_BIND_TYPE_IMPLICIT; - PyObject *t1 = PyTuple_GetItem(tup, 1); + PyObject *t1 = PyTuple_GET_ITEM(tup, 1); if (!PyUnicode_Check(t1)) { PyErr_SetString(PyExc_TypeError, "hypothesis variable must be a string"); + FreeHypos(hypos, i); return NULL; } hypos[i].cid = PyUnicode_AsPgfText(t1); - PyObject *t2 = PyTuple_GetItem(tup, 2); + PyObject *t2 = PyTuple_GET_ITEM(tup, 2); if (!PyObject_TypeCheck(t2, &pgf_TypeType)) { PyErr_SetString(PyExc_TypeError, "hypothesis type must be a Type"); + FreePgfText(hypos[i].cid); + FreeHypos(hypos, i); return NULL; } hypos[i].type = (PgfType) t2; @@ -113,37 +108,29 @@ PySequence_AsHypos(PyObject *pyseq, Py_ssize_t *n_hypos) } PyObject * -PyList_FromHypos(PgfTypeHypo *hypos, const size_t n_hypos) +PyTuple_FromHypos(PgfTypeHypo *hypos, const size_t n_hypos) { - PyObject *pylist = PyList_New(n_hypos); - if (pylist == NULL) { + PyObject *pytuple = PyTuple_New(n_hypos); + if (pytuple == NULL) { return NULL; } for (size_t i = 0; i < n_hypos; i++) { - // HypoObject *hypo = PyObject_New(HypoObject, &pgf_HypoType); - // hypo->bind_type = hypos[i].bind_type == 0 ? Py_True : Py_False; - // hypo->cid = PyUnicode_FromStringAndSize(hypos[i].cid->text, hypos[i].cid->size); - // hypo->type = (TypeObject *)hypos[i].type; - // Py_INCREF(hypo->bind_type); - // Py_INCREF(hypo->type); - // PyList_SetItem(pylist, i, (PyObject *)hypo); - PyObject *tup = PyTuple_New(3); PyObject *bt = hypos[i].bind_type == 0 ? Py_True : Py_False; - PyTuple_SetItem(tup, 0, bt); - PyTuple_SetItem(tup, 1, PyUnicode_FromStringAndSize(hypos[i].cid->text, hypos[i].cid->size)); - PyTuple_SetItem(tup, 2, (PyObject *)hypos[i].type); + PyTuple_SET_ITEM(tup, 0, bt); + PyTuple_SET_ITEM(tup, 1, PyUnicode_FromStringAndSize(hypos[i].cid->text, hypos[i].cid->size)); + PyTuple_SET_ITEM(tup, 2, (PyObject *)hypos[i].type); Py_INCREF(bt); Py_INCREF(hypos[i].type); - PyList_SetItem(pylist, i, tup); + PyTuple_SET_ITEM(pytuple, i, tup); } if (PyErr_Occurred()) { - Py_DECREF(pylist); + Py_DECREF(pytuple); return NULL; } - return pylist; + return pytuple; } PgfPrintContext * @@ -331,7 +318,7 @@ dtyp(PgfUnmarshaller *this, int n_hypos, PgfTypeHypo *hypos, PgfText *cat, int n { TypeObject *pytype = (TypeObject *)pgf_TypeType.tp_alloc(&pgf_TypeType, 0); - pytype->hypos = PySequence_Tuple(PyList_FromHypos(hypos, n_hypos)); + pytype->hypos = PyTuple_FromHypos(hypos, n_hypos); pytype->name = PyUnicode_FromStringAndSize(cat->text, cat->size); pytype->exprs = PyTuple_New(n_exprs); for (int i = 0; i < n_exprs; i++) { diff --git a/src/runtime/python/ffi.h b/src/runtime/python/ffi.h index 1d3ca3f25..15e07eb13 100644 --- a/src/runtime/python/ffi.h +++ b/src/runtime/python/ffi.h @@ -21,7 +21,7 @@ PgfText *PyUnicode_AsPgfText(PyObject *pystr); PyObject *PyUnicode_FromPgfText(PgfText *text); PgfTypeHypo *PySequence_AsHypos(PyObject *pyseq, Py_ssize_t *n_hypos); -PyObject *PyList_FromHypos(PgfTypeHypo *hypos, const size_t n_hypos); +PyObject *PyTuple_FromHypos(PgfTypeHypo *hypos, const size_t n_hypos); PgfPrintContext *PyList_AsPgfPrintContext(PyObject *pylist); diff --git a/src/runtime/python/pypgf.c b/src/runtime/python/pypgf.c index 0354904d6..e346c3796 100644 --- a/src/runtime/python/pypgf.c +++ b/src/runtime/python/pypgf.c @@ -148,7 +148,7 @@ PGF_categoryContext(PGFObject *self, PyObject *args) Py_RETURN_NONE; } - PyObject *contexts = PyList_FromHypos(hypos, n_hypos); + PyObject *contexts = PyTuple_FromHypos(hypos, n_hypos); for (size_t i = 0; i < n_hypos; i++) { free(hypos[i].cid); diff --git a/src/runtime/python/tests/test_basic.py b/src/runtime/python/tests/test_basic.py index bd5477162..bfe34661f 100644 --- a/src/runtime/python/tests/test_basic.py +++ b/src/runtime/python/tests/test_basic.py @@ -104,10 +104,10 @@ def test_functionsByCat_non_existent(PGF): assert PGF.functionsByCat("X") == [] def test_categoryContext_1(PGF): - assert PGF.categoryContext("N") == [] + assert PGF.categoryContext("N") == () def test_categoryContext_2(PGF): - assert PGF.categoryContext("S") == [] + assert PGF.categoryContext("S") == () def test_categoryContext_3(PGF): cxt = PGF.categoryContext("P") diff --git a/src/runtime/python/tests/test_transactions.py b/src/runtime/python/tests/test_transactions.py index e5c1b8fef..143d00c3d 100644 --- a/src/runtime/python/tests/test_transactions.py +++ b/src/runtime/python/tests/test_transactions.py @@ -89,7 +89,8 @@ def test_extended_categories(gr2): assert gr2.categories == ["Float","Int","N","P","Q","S","String"] def test_extended_category_context(gr2): - assert gr2.categoryContext("Q") == [(BIND_TYPE_EXPLICIT, "x", ty)] + print(gr2.categoryContext("Q")) + assert gr2.categoryContext("Q") == ((BIND_TYPE_EXPLICIT, "x", ty),) def test_extended_function_type(gr2): assert gr2.functionType("foo") == ty @@ -112,7 +113,7 @@ def test_branched_categories(gr3): assert gr3.categories == ["Float","Int","N","P","R","S","String"] def test_branched_category_context(gr3): - assert gr3.categoryContext("R") == [(BIND_TYPE_EXPLICIT, "x", ty)] + assert gr3.categoryContext("R") == ((BIND_TYPE_EXPLICIT, "x", ty),) def test_branched_function_type(gr3): assert gr3.functionType("bar") == ty