Browse Source

[1028] fiexed leak in RRset.get_rdata() as well as other minor bugs
like missing NULL check or exception safety. also as a cleanup got rid
of unnecessary reinterpret_cast.

JINMEI Tatuya 13 years ago
parent
commit
b5d072cfe2
2 changed files with 25 additions and 15 deletions
  1. 18 15
      src/lib/dns/python/rrset_python.cc
  2. 7 0
      src/lib/dns/python/tests/rrset_python_test.py

+ 18 - 15
src/lib/dns/python/rrset_python.cc

@@ -63,7 +63,7 @@ PyObject* RRset_toText(s_RRset* self);
 PyObject* RRset_str(PyObject* self);
 PyObject* RRset_toWire(s_RRset* self, PyObject* args);
 PyObject* RRset_addRdata(s_RRset* self, PyObject* args);
-PyObject* RRset_getRdata(s_RRset* self);
+PyObject* RRset_getRdata(PyObject* po_self, PyObject*);
 PyObject* RRset_removeRRsig(s_RRset* self);
 
 // TODO: iterator?
@@ -94,7 +94,7 @@ PyMethodDef RRset_methods[] = {
       "returned" },
     { "add_rdata", reinterpret_cast<PyCFunction>(RRset_addRdata), METH_VARARGS,
       "Adds the rdata for one RR to the RRset.\nTakes an Rdata object as an argument" },
-    { "get_rdata", reinterpret_cast<PyCFunction>(RRset_getRdata), METH_NOARGS,
+    { "get_rdata", RRset_getRdata, METH_NOARGS,
       "Returns a List containing all Rdata elements" },
     { "remove_rrsig", reinterpret_cast<PyCFunction>(RRset_removeRRsig), METH_NOARGS,
       "Clears the list of RRsigs for this RRset" },
@@ -291,22 +291,26 @@ RRset_addRdata(s_RRset* self, PyObject* args) {
 }
 
 PyObject*
-RRset_getRdata(s_RRset* self) {
-    PyObject* list = PyList_New(0);
-
-    RdataIteratorPtr it = self->cppobj->getRdataIterator();
+RRset_getRdata(PyObject* po_self, PyObject*) {
+    const s_RRset* const self = static_cast<s_RRset*>(po_self);
 
     try {
-        for (; !it->isLast(); it->next()) {
-            const rdata::Rdata *rd = &it->getCurrent();
-            if (PyList_Append(list,
-                    createRdataObject(createRdata(self->cppobj->getType(),
-                                      self->cppobj->getClass(), *rd))) == -1) {
-                Py_DECREF(list);
-                return (NULL);
+        PyObjectContainer list_container(PyList_New(0));
+
+        for (RdataIteratorPtr it = self->cppobj->getRdataIterator();
+             !it->isLast(); it->next()) {
+            if (PyList_Append(list_container.get(),
+                              PyObjectContainer(
+                                  createRdataObject(
+                                      createRdata(self->cppobj->getType(),
+                                                  self->cppobj->getClass(),
+                                                  it->getCurrent()))).get())
+                == -1) {
+                isc_throw(PyCPPWrapperException, "PyList_Append failed, "
+                          "probably due to short memory");
             }
         }
-        return (list);
+        return (list_container.release());
     } catch (const exception& ex) {
         const string ex_what =
             "Unexpected failure getting rrset Rdata: " +
@@ -316,7 +320,6 @@ RRset_getRdata(s_RRset* self) {
         PyErr_SetString(PyExc_SystemError,
                         "Unexpected failure getting rrset Rdata");
     }
-    Py_DECREF(list);
     return (NULL);
 }
 

+ 7 - 0
src/lib/dns/python/tests/rrset_python_test.py

@@ -17,6 +17,7 @@
 # Tests for the rrtype part of the pydnspp module
 #
 
+import sys
 import unittest
 import os
 from pydnspp import *
@@ -110,6 +111,12 @@ class TestModuleSpec(unittest.TestCase):
                 ]
         self.assertEqual(rdata, self.rrset_a.get_rdata())
         self.assertEqual([], self.rrset_a_empty.get_rdata())
+
+        # We always make a new deep copy in get_rdata(), so the reference
+        # count of the returned list and its each item should be 1; otherwise
+        # they would leak.
+        self.assertEqual(1, sys.getrefcount(self.rrset_a.get_rdata()))
+        self.assertEqual(1, sys.getrefcount(self.rrset_a.get_rdata()[0]))
         
 if __name__ == '__main__':
     unittest.main()