fixed some memory leakage problems #3
Conversation
colinpattison
left a comment
colinpattison
left a comment
There was a problem hiding this comment.
Hey Nima - thanks for the PR. A few changes highlighted around styling. I'm also going to request review from Rich on the subscript changes. I've reworked that code a bit on another branch to improve re-use so there will be some changes to it coming up soon
C
| @@ -179,17 +189,21 @@ serializeBytes (PyObject* o, void*& v, size_t& l) | |||
| PyObject* | |||
| deserializeObject (void* buf, size_t len) | |||
There was a problem hiding this comment.
This fix is correct however it needs made to pickleLoads and this function should be calling pickleLoads as the code is currently duplicated. Can you add this DECREF to pickeLoads and have this function call pickleLoads
rebuild.sh
Outdated
| @@ -0,0 +1,7 @@ | |||
| #!/bin/bash -x | |||
There was a problem hiding this comment.
Please don't commit this script
| #endif | ||
|
|
||
| return PyObject_CallMethodObjArgs (PICKLE_MODULE, | ||
| PyObject* res = PyObject_CallMethodObjArgs (PICKLE_MODULE, |
There was a problem hiding this comment.
Hey - you need to add the same code here for the loads method name, and then subsequently decref it at the end
C
| } | ||
|
|
||
|
|
||
| static char buffer[700000]; |
There was a problem hiding this comment.
Hi, why did you make the buffer static out here I don't see why there was a memory leak with using the stack buffer?
There was a problem hiding this comment.
This particular change was made for performance enhancing reasons.
3 participants
No description provided.