Fix repr and str for lists with cycles

Functions which format a string from linked list (repr and str) need to handle the case where the list cyclically references itself. Failing to detect this scenario leads to infinite recursion (which is detected by Python interpreter and causes an exception). Two distinct list layouts can trigger this issue: - a list which contains a reference to itself in one of the nodes - a node which contains a reference to itself (which might be indirect) Both cases are handled by calling Py_ReprEnter/Py_ReprLeave and terminating infinite recursion in repr and str. Closes issue #10.
ajakubek · Aug 25, 2019 · 01603ae · 01603ae
1 parent 76a050e
commit 01603ae
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 29 deletions.
diff --git a/CHANGES b/CHANGES
@@ -1,5 +1,8 @@
 * llist-0.7 (unreleased)
 
+  - fixed repr() and str() for self-referencing lists and nodes
+    (closes issue #10)
+
 -----------------------------------------------------------------------
 
 * llist-0.6 (2018-06-30)

diff --git a/src/dllist.c b/src/dllist.c
@@ -143,6 +143,9 @@ static PyObject* dllistnode_to_string(DLListNodeObject* self,
 
     assert(fmt_func != NULL);
 
+    if (Py_ReprEnter((PyObject*)self) > 0)
+        return Py23String_FromString("dllistnode(<...>)");
+
     str = Py23String_FromString(prefix);
     if (str == NULL)
         goto str_alloc_error;
@@ -157,11 +160,16 @@ static PyObject* dllistnode_to_string(DLListNodeObject* self,
         goto str_alloc_error;
     Py23String_ConcatAndDel(&str, tmp_str);
 
+    Py_ReprLeave((PyObject*)self);
+
     return str;
 
 str_alloc_error:
     Py_XDECREF(str);
     PyErr_SetString(PyExc_RuntimeError, "Failed to create string");
+
+    Py_ReprLeave((PyObject*)self);
+
     return NULL;
 }
 
@@ -478,6 +486,9 @@ static PyObject* dllist_to_string(DLListObject* self,
 
     assert(fmt_func != NULL);
 
+    if (Py_ReprEnter((PyObject*)self) > 0)
+        return Py23String_FromString("dllist(<...>)");
+
     if (self->first == Py_None)
     {
         str = Py23String_FromString("dllist()");
@@ -515,12 +526,17 @@ static PyObject* dllist_to_string(DLListObject* self,
         goto str_alloc_error;
     Py23String_ConcatAndDel(&str, tmp_str);
 
+    Py_ReprLeave((PyObject*)self);
+
     return str;
 
 str_alloc_error:
     Py_XDECREF(str);
     Py_XDECREF(comma_str);
     PyErr_SetString(PyExc_RuntimeError, "Failed to create string");
+
+    Py_ReprLeave((PyObject*)self);
+
     return NULL;
 }
 

diff --git a/src/sllist.c b/src/sllist.c
@@ -176,61 +176,55 @@ static PyObject* sllistnode_new(PyTypeObject* type,
 }
 
 
-
-static PyObject* sllistnode_repr(SLListNodeObject* self)
+/* Convenience function for formatting list node to a string.
+ * Pass PyObject_Repr or PyObject_Str in the fmt_func argument. */
+static PyObject* sllistnode_to_string(SLListNodeObject* self,
+                                      reprfunc fmt_func,
+                                      const char* prefix,
+                                      const char* suffix)
 {
     PyObject* str = NULL;
     PyObject* tmp_str;
 
-    str = Py23String_FromString("<sllistnode(");
+    if (Py_ReprEnter((PyObject*)self) > 0)
+        return Py23String_FromString("sllistnode(<...>)");
+
+    str = Py23String_FromString(prefix);
     if (str == NULL)
         goto str_alloc_error;
 
-    tmp_str = PyObject_Repr(self->value);
+    tmp_str = fmt_func(self->value);
     if (tmp_str == NULL)
         goto str_alloc_error;
     Py23String_ConcatAndDel(&str, tmp_str);
 
-    tmp_str = Py23String_FromString(")>");
+    tmp_str = Py23String_FromString(suffix);
     if (tmp_str == NULL)
         goto str_alloc_error;
     Py23String_ConcatAndDel(&str, tmp_str);
 
+    Py_ReprLeave((PyObject*)self);
+
     return str;
 
 str_alloc_error:
     Py_XDECREF(str);
     PyErr_SetString(PyExc_RuntimeError, "Failed to create string");
+
+    Py_ReprLeave((PyObject*)self);
+
     return NULL;
 }
 
-static PyObject* sllistnode_str(SLListNodeObject* self)
+static PyObject* sllistnode_repr(SLListNodeObject* self)
 {
-    PyObject* str = NULL;
-    PyObject* tmp_str;
-
-    str = Py23String_FromString("sllistnode(");
-    if (str == NULL)
-        goto str_alloc_error;
-
-    tmp_str = PyObject_Str(self->value);
-    if (tmp_str == NULL)
-        goto str_alloc_error;
-    Py23String_ConcatAndDel(&str, tmp_str);
-
-    tmp_str = Py23String_FromString(")");
-    if (tmp_str == NULL)
-        goto str_alloc_error;
-    Py23String_ConcatAndDel(&str, tmp_str);
-
-    return str;
-
-str_alloc_error:
-    Py_XDECREF(str);
-    PyErr_SetString(PyExc_RuntimeError, "Failed to create string");
-    return NULL;
+    return sllistnode_to_string(self, PyObject_Repr, "<sllistnode(", ")>");
 }
 
+static PyObject* sllistnode_str(SLListNodeObject* self)
+{
+    return sllistnode_to_string(self, PyObject_Str, "sllistnode(", ")");
+}
 
 
 
@@ -1441,6 +1435,9 @@ static PyObject* sllist_to_string(SLListObject* self,
 
     assert(fmt_func != NULL);
 
+    if (Py_ReprEnter((PyObject*)self) > 0)
+        return Py23String_FromString("sllist(<...>)");
+
     if (self->first == Py_None)
     {
         str = Py23String_FromString("sllist()");
@@ -1478,12 +1475,17 @@ static PyObject* sllist_to_string(SLListObject* self,
         goto str_alloc_error;
     Py23String_ConcatAndDel(&str, tmp_str);
 
+    Py_ReprLeave((PyObject*)self);
+
     return str;
 
 str_alloc_error:
     Py_XDECREF(str);
     Py_XDECREF(comma_str);
     PyErr_SetString(PyExc_RuntimeError, "Failed to create string");
+
+    Py_ReprLeave((PyObject*)self);
+
     return NULL;
 }
 

diff --git a/tests/llist_test.py b/tests/llist_test.py
@@ -87,6 +87,31 @@ def test_node_repr(self):
         c = sllist(['abc', None]).first
         self.assertEqual(repr(c), '<sllistnode(\'abc\')>')
 
+    def test_str_recursive_list(self):
+        ll = sllist()
+        ll.append(sllistnode(ll))
+        self.assertEqual(str(ll), 'sllist([sllist(<...>)])')
+
+    def test_str_recursive_node(self):
+        ll = self.make_recursive_node_list()
+        self.assertEqual(str(ll), 'sllist([sllistnode(sllistnode(<...>))])')
+
+    def test_repr_recursive_list(self):
+        ll = sllist()
+        ll.append(sllistnode(ll))
+        self.assertEqual(repr(ll), 'sllist([sllist(<...>)])')
+
+    def test_repr_recursive_node_list(self):
+        ll = self.make_recursive_node_list()
+        self.assertEqual(repr(ll), 'sllist([<sllistnode(sllistnode(<...>))>])')
+
+    def make_recursive_node_list(self):
+        ll = sllist()
+        node = sllistnode()
+        node.value = node
+        ll.append(node)
+        return ll
+
     def test_cmp(self):
         a = sllist(py23_xrange(0, 1100))
         b = sllist(py23_xrange(0, 1101))
@@ -1035,6 +1060,31 @@ def test_node_repr(self):
         c = dllist(['abc', None]).first
         self.assertEqual(repr(c), '<dllistnode(\'abc\')>')
 
+    def test_str_recursive_list(self):
+        ll = dllist()
+        ll.append(dllistnode(ll))
+        self.assertEqual(str(ll), 'dllist([dllist(<...>)])')
+
+    def test_str_recursive_node(self):
+        ll = self.make_recursive_node_list()
+        self.assertEqual(str(ll), 'dllist([dllistnode(dllistnode(<...>))])')
+
+    def test_repr_recursive_list(self):
+        ll = dllist()
+        ll.append(dllistnode(ll))
+        self.assertEqual(repr(ll), 'dllist([dllist(<...>)])')
+
+    def test_repr_recursive_node(self):
+        ll = self.make_recursive_node_list()
+        self.assertEqual(repr(ll), 'dllist([<dllistnode(dllistnode(<...>))>])')
+
+    def make_recursive_node_list(self):
+        ll = dllist()
+        node = dllistnode()
+        node.value = node
+        ll.append(node)
+        return ll
+
     def test_cmp(self):
         a = dllist(py23_xrange(0, 1100))
         b = dllist(py23_xrange(0, 1101))