You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by "Gustavo Niemeyer (JIRA)" <ji...@apache.org> on 2009/11/30 23:23:20 UTC
[jira] Created: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
TODO pondering about allocation behavior in zkpython may be removed
-------------------------------------------------------------------
Key: ZOOKEEPER-600
URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
Project: Zookeeper
Issue Type: Task
Reporter: Gustavo Niemeyer
Priority: Trivial
I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
Index: src/contrib/zkpython/src/c/zookeeper.c
===================================================================
--- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
+++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
@@ -774,8 +774,6 @@
static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
{
- // TO DO: Does Python copy the string or the reference? If it's the former
- // we should free the String_vector
int zkhid;
char *path;
PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Gustavo Niemeyer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12783933#action_12783933 ]
Gustavo Niemeyer commented on ZOOKEEPER-600:
--------------------------------------------
Patrick, thanks for pointing me to the conventions, and sorry for missing it earlier.
I'll give it a shot and submit a patch soon.
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12783919#action_12783919 ]
Henry Robinson commented on ZOOKEEPER-600:
------------------------------------------
Correct - my concern is over whether PyString_FromString copies the C string it is given. If it does not, then a String_vector is allocated by zoo_wgetchildren and never freed.
http://docs.python.org/c-api/string.html hints that a copy is made, and therefore the memory needs freeing.
This would be relatively easy to check by memsetting all strings in the String_vector to 'A' after the copy, and then checking to see if the Python-side strings are altered. Alternatively, you could check the Python C API source - it should be fairly clear what the answer is.
Thanks for picking this up!
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12784352#action_12784352 ]
Henry Robinson commented on ZOOKEEPER-600:
------------------------------------------
Patch applies fine for me and tests all pass - looks good, thanks Gustavo! I'll open a JIRA for the other issues.
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
> Attachments: deallocate-vector.patch
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Gustavo Niemeyer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gustavo Niemeyer updated ZOOKEEPER-600:
---------------------------------------
Attachment: deallocate-vector.patch
The attached patch should fix this problem. It also reuses existing code, and fixes a few other issues around the former problem, with return values not being properly checked for errors.
I'm afraid there are several instances of variables which are not checked for error conditions in the module, unfortunately. :-( I'm not going to try fixing these in this JIRA, though.
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
> Attachments: deallocate-vector.patch
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Hadoop QA (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12789476#action_12789476 ]
Hadoop QA commented on ZOOKEEPER-600:
-------------------------------------
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12426565/deallocate-vector.patch
against trunk revision 888216.
+1 @author. The patch does not contain any @author tags.
-1 tests included. The patch doesn't appear to include any new or modified tests.
Please justify why no tests are needed for this patch.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 findbugs. The patch does not introduce any new Findbugs warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 core tests. The patch passed core unit tests.
+1 contrib tests. The patch passed contrib unit tests.
Test results: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/22/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/22/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h7.grid.sp2.yahoo.net/22/console
This message is automatically generated.
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
> Attachments: deallocate-vector.patch
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Gustavo Niemeyer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12791901#action_12791901 ]
Gustavo Niemeyer commented on ZOOKEEPER-600:
--------------------------------------------
I believe it's ready for integration.
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
> Attachments: deallocate-vector.patch
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Hunt updated ZOOKEEPER-600:
-----------------------------------
Component/s: contrib-bindings
Affects Version/s: 3.2.1
Fix Version/s: 3.3.0
Issue Type: Bug (was: Task)
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Steven Cheng (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12783916#action_12783916 ]
Steven Cheng commented on ZOOKEEPER-600:
----------------------------------------
Hi Gustavo, I think it's talking about the struct String_vector strings that gets copied at the end of the function.
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12792483#action_12792483 ]
Hudson commented on ZOOKEEPER-600:
----------------------------------
Integrated in ZooKeeper-trunk #634 (See [http://hudson.zones.apache.org/hudson/job/ZooKeeper-trunk/634/])
. TODO pondering about allocation behavior in zkpython may be removed (gustavo via mahadev)
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
> Attachments: deallocate-vector.patch
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mahadev konar updated ZOOKEEPER-600:
------------------------------------
Resolution: Fixed
Hadoop Flags: [Reviewed]
Status: Resolved (was: Patch Available)
great ... I just committed this. thanks gustavo.
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
> Attachments: deallocate-vector.patch
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12792069#action_12792069 ]
Henry Robinson commented on ZOOKEEPER-600:
------------------------------------------
Yes, this looks good to me - I'd like to see a test included, but we have no infrastructure for C-side tests which this would probably need. Can be committed as far as I am concerned. Thanks Gustavo!
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
> Attachments: deallocate-vector.patch
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Mahadev konar (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12791681#action_12791681 ]
Mahadev konar commented on ZOOKEEPER-600:
-----------------------------------------
henry/gustavo,
is this ready to commit?
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
> Attachments: deallocate-vector.patch
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Gustavo Niemeyer (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12783931#action_12783931 ]
Gustavo Niemeyer commented on ZOOKEEPER-600:
--------------------------------------------
Ah, I see.
Yes, PyString_FromString will definitely copy the string over, so the strings.data array is leaking if it has data allocated dynamically.
In addition to this, PyString_FromString and PyList_New are both allocating memory, and thus they may fail to return a proper object. This isn't being checked currently.
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12783903#action_12783903 ]
Patrick Hunt commented on ZOOKEEPER-600:
----------------------------------------
Hi Gustavo, thanks for the submit. I need to point out that we require submissions via patch file, details of which you can find here:
http://wiki.apache.org/hadoop/ZooKeeper/HowToContribute
(use svn diff to create a patch, attach it to this jira using "attach file" link on the left hand side of this page)
The reason for this is that we need to capture your acceptance of the license grant to ASF. Otw we cannot
accept the patch (for legal reasons). Also our automated workflow checks submissions and such, it's triggered
by your attaching the file, then clicking on "submit patch". Thanks for your patience.
If you could attach you change as a patch file that would be great.
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Assigned: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Hunt reassigned ZOOKEEPER-600:
--------------------------------------
Assignee: Gustavo Niemeyer
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Task
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Patrick Hunt (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12784056#action_12784056 ]
Patrick Hunt commented on ZOOKEEPER-600:
----------------------------------------
no worries, we don't expect first time contributors to know everything. ;-) thanks for the interest.
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (ZOOKEEPER-600) TODO pondering about allocation
behavior in zkpython may be removed
Posted by "Henry Robinson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Henry Robinson updated ZOOKEEPER-600:
-------------------------------------
Status: Patch Available (was: Open)
Marking as patch submitted so that hudsonqabot can take a look.
> TODO pondering about allocation behavior in zkpython may be removed
> -------------------------------------------------------------------
>
> Key: ZOOKEEPER-600
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-600
> Project: Zookeeper
> Issue Type: Bug
> Components: contrib-bindings
> Affects Versions: 3.2.1
> Reporter: Gustavo Niemeyer
> Assignee: Gustavo Niemeyer
> Priority: Trivial
> Fix For: 3.3.0
>
> Attachments: deallocate-vector.patch
>
>
> I suppose the TODO below is referring to the "path" variable which is passed in as an output variable to PyArg_ParseTuple right below. The TODO may be removed, since the code is right. Code using PyArg_ParseTuple will borrow the reference from the calling code, since there's a stack behind the call to the enclosing function (pyzoo_get_children in this case) which won't go away until the function returns.
> Index: src/contrib/zkpython/src/c/zookeeper.c
> ===================================================================
> --- src/contrib/zkpython/src/c/zookeeper.c (revision 885582)
> +++ src/contrib/zkpython/src/c/zookeeper.c (working copy)
> @@ -774,8 +774,6 @@
>
> static PyObject *pyzoo_get_children(PyObject *self, PyObject *args)
> {
> - // TO DO: Does Python copy the string or the reference? If it's the former
> - // we should free the String_vector
> int zkhid;
> char *path;
> PyObject *watcherfn = Py_None;
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.