Attached patch addresses the following:

1. Formatting redone to be (nearly) consistent
2. Comments added to every function
3. zookeeper.c reorganised logically
4. Py_None now reference counted correctly (see ZOOKEEPER-742)
5. Memory allocations now checked, and general error handling greatly improved.
6. A variety of small bugs and typos fixed

The result is hopefully a much more stable zkpython. This patch will look like a rewrite - there are lots of changes. Apologies to the reviewer in advance!

(I am happy for this patch to be used by the ASF, but the button is not available to be checked).

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12442027/ZOOKEEPER-631.patch
against trunk revision 934312.

+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-h1.grid.sp2.yahoo.net/57/testReport/
Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/57/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-h1.grid.sp2.yahoo.net/57/console

This message is automatically generated.

The existing tests are the ones that validate this patch. To test the Py_None and memory allocation issues is hard because in the first case the GC behaviour is hard to force and in the second we would have to stub out calloc(..) somehow!

Committed to 3.3 branch and trunk. Thanks Henry!

I verified by inspecting the code changes, running the unit tests, but I also used the zk-smoketest/zk-latencies
http://github.com/phunt/zk-smoketest to verify the changed code.

One suggestion - next time create multiple JIRAs with individual patches rather than one big patch. Would
make it much easier to review (say reformatting only, docs, acl/memory fixes. something like that.)