Well actually I count 5 non-error-condition leaks: 1. free(depList) at the end of depClassFreezeRaw() in dep_freeze.c 2. Py_DECREF(tagObj) and Py_DECREF(classMembers) inside the loop depSetFreeze() in dep_freeze.c 3. free(result) at the end of pack() in pack.c 4. Two more in stream_set.c
The missing free() are pretty obvious but I'd really like review of the refcount ones because I'm pretty new to Python and its C bindings. -----Original Message----- From: Foresight-devel [mailto:[email protected]] On Behalf Of Michael K. Johnson Sent: Monday, March 04, 2013 6:17 PM To: Foresight Linux Development Subject: [Foresight-devel] Re: Conary/rAPA bugs and fixes - where to post them? On Mon, Mar 04, 2013 at 06:47:10AM -0500, Michael K. Johnson wrote: > Finally, I think you forgot to post the patches, unless I have somehow > accidentally enabled a "strip attachments from posts" feature in > setting up this list in mailman... OK, mailman seems to default (at least, I didn't change from the defaults intentionally) to stripping non-whitelisted attachment types. It allows text/plain and text/x-patch among others. Uri sent the Conary patch to me directly. Here it is inline: diff -ruN rpathsync-conary-fb64f746afaf/conary/lib/ext/dep_freeze.c rpathsync-conary-fb64f746afaf_new/conary/lib/ext/dep_freeze.c --- rpathsync-conary-fb64f746afaf/conary/lib/ext/dep_freeze.c 2012-04-11 18:14:08.000000000 +0300 +++ rpathsync-conary-fb64f746afaf_new/conary/lib/ext/dep_freeze.c 2013-03-03 10:46:22.000000000 +0200 @@ -300,7 +300,6 @@ ((struct depList *) b)->className); } -/* This leaks memory on error. Oh well. */ static int depClassFreezeRaw(PyObject * tagObj, PyObject * dict, char ** resultPtr, int * resultSizePtr) { PyObject * depObjList, * tuple; @@ -333,6 +332,11 @@ } depList = malloc(depCount * sizeof(*depList)); + if (!depList) + return -1; + + memset(depList,0,depCount * sizeof(*depList)); + for (i = 0; i < depCount; i++) { tuple = PyList_GET_ITEM(depObjList, i); if (!PYBYTES_CheckExact(PyTuple_GET_ITEM(tuple, 0))) { @@ -350,15 +354,17 @@ qsort(depList, depCount, sizeof(*depList), depListSort); totalSize = 0; + rc=0; for (i = 0; i < depCount; i++) { if (!(nameObj = PyObject_GetAttrString(depList[i].dep, "name"))) { - free(depList); - return -1; + rc=-1; + break; } if (!(flagsObj = PyObject_GetAttrString(depList[i].dep, "flags"))) { - free(depList); - return -1; + Py_DECREF(nameObj); + rc=-1; + break; } rc = depFreezeRaw(nameObj, flagsObj, &depList[i].frz, &depList[i].frzSize); @@ -367,13 +373,20 @@ Py_DECREF(flagsObj); if (rc == -1) { - free(depList); - return -1; + break; } totalSize += depList[i].frzSize; } + if ( rc==-1 ) { + for (i = 0; i < depCount; i++) { + free(depList[i].frz); + } + free(depList); + return -1; + } + /* 15 leaves plenty of room for the tag integer and the # */ result = malloc((depCount * 15) + totalSize); next = result; @@ -393,6 +406,7 @@ *resultPtr = result; *resultSizePtr = next - result; + free(depList); return 0; } @@ -482,6 +496,9 @@ } totalSize += members[i].frzSize + 1; + + Py_DECREF(tagObj); + Py_DECREF(classMembers); } Py_DECREF(memberList); diff -ruN rpathsync-conary-fb64f746afaf/conary/lib/ext/pack.c rpathsync-conary-fb64f746afaf_new/conary/lib/ext/pack.c --- rpathsync-conary-fb64f746afaf/conary/lib/ext/pack.c 2012-04-11 18:14:08.000000000 +0300 +++ rpathsync-conary-fb64f746afaf_new/conary/lib/ext/pack.c 2013-02-21 06:33:06.000000000 +0200 @@ -140,6 +140,9 @@ } result = malloc(strLen); + if (!result) + return NULL; + argNum = 1; strLen = 0; formatPtr = format + 1; @@ -168,11 +171,13 @@ formatPtr++; } else if (isdigit(*formatPtr)) { if (getSize(&formatPtr, &i)) { + free(result); return NULL; } } else { PyErr_SetString(PyExc_RuntimeError, "internal pack error 1"); + free(result); return NULL; } @@ -184,11 +189,13 @@ default: PyErr_SetString(PyExc_RuntimeError, "internal pack error 2"); + free(result); return NULL; } } resultObj = PYBYTES_FromStringAndSize(result, strLen); + free(result); return resultObj; } diff -ruN rpathsync-conary-fb64f746afaf/conary/lib/ext/streams_set.c rpathsync-conary-fb64f746afaf_new/conary/lib/ext/streams_set.c --- rpathsync-conary-fb64f746afaf/conary/lib/ext/streams_set.c 2012-04-11 18:14:08.000000000 +0300 +++ rpathsync-conary-fb64f746afaf_new/conary/lib/ext/streams_set.c 2013-02-27 08:06:25.000000000 +0200 @@ -66,7 +66,12 @@ /* StreamSetDef Implementation */ static void StreamSetDef_Dealloc(PyObject * self) { + int i; StreamSetDefObject * ssd = (StreamSetDefObject *) self; + for (i=0; i<ssd->tagCount; ++i) { + Py_CLEAR(ssd->tags[i].name); + Py_CLEAR(ssd->tags[i].type); + } free(ssd->tags); Py_TYPE(self)->tp_free(self); } @@ -112,6 +117,8 @@ Py_INCREF(streamType); } + Py_DECREF(items); + /* simple bubble sort */ for (i = 0; i < ssd->tagCount - 1; i++) { for (j = 0; j < ssd->tagCount - i - 1; j++) { If I read this correctly, all the changes except for the the stream_set.c change are for out-of-memory conditions. Am I missing anything? Thanks _______________________________________________ Foresight-devel mailing list [email protected] https://lists.foresightlinux.org/mailman/listinfo/foresight-devel _______________________________________________ Foresight-devel mailing list [email protected] https://lists.foresightlinux.org/mailman/listinfo/foresight-devel
