[jira] Commented: (ZOOKEEPER-600) TODO pondering about allocation behavior in zkpython may be removed
[
https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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] Commented: (ZOOKEEPER-600) TODO pondering about allocation behavior in zkpython may be removed
[
https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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] Commented: (ZOOKEEPER-600) TODO pondering about allocation behavior in zkpython may be removed
[
https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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
[
https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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
[
https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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
[
https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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] Commented: (ZOOKEEPER-600) TODO pondering about allocation behavior in zkpython may be removed
[
https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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
[
https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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
[
https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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
[
https://issues.apache.org/jira/browse/ZOOKEEPER-600?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=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.
