From: JINMEI Tatuya <jinmei@isc.org>
Date: Wed, 26 Oct 2011 05:42:12 +0000 (-0700)
Subject: [1028] fiexed leak in RRset.get_rdata() as well as other minor bugs
X-Git-Tag: perftcpdns_before_epoll~66^2~1^2~10
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b5d072cfe24be6ad1636dfdb50405ff32473a413;p=thirdparty%2Fkea.git

[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.
---

diff --git a/src/lib/dns/python/rrset_python.cc b/src/lib/dns/python/rrset_python.cc
index 9fc3d79166..73a19e7770 100644
--- a/src/lib/dns/python/rrset_python.cc
+++ b/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);
 }
 
diff --git a/src/lib/dns/python/tests/rrset_python_test.py b/src/lib/dns/python/tests/rrset_python_test.py
index e0eab4a246..de475a766d 100644
--- a/src/lib/dns/python/tests/rrset_python_test.py
+++ b/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()