[ https://issues.apache.org/jira/browse/ZOOKEEPER-311?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12737148#action_12737148 ]
Hadoop QA commented on ZOOKEEPER-311: ------------------------------------- +1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12413380/ZOOKEEPER-311.patch against trunk revision 798038. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. +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-vesta.apache.org/160/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/160/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Zookeeper-Patch-vesta.apache.org/160/console This message is automatically generated. > handle small path lengths in zoo_create() > ----------------------------------------- > > Key: ZOOKEEPER-311 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-311 > Project: Zookeeper > Issue Type: Improvement > Components: c client > Affects Versions: 3.0.0, 3.0.1, 3.1.0, 3.1.1, 3.2.0 > Reporter: Chris Darroch > Assignee: Chris Darroch > Priority: Minor > Fix For: 3.2.1, 3.3.0 > > Attachments: ZOOKEEPER-311.patch, ZOOKEEPER-311.patch > > > The synchronous completion for zoo_create() contains the following code:\\ > {noformat} > if (sc->u.str.str_len > strlen(res.path)) { > len = strlen(res.path); > } else { > len = sc->u.str.str_len-1; > } > if (len > 0) { > memcpy(sc->u.str.str, res.path, len); > sc->u.str.str[len] = '\0'; > } > {noformat} > In the case where the max_realpath_len argument to zoo_create() is 0, none of > this code executes, which is OK. In the case where max_realpath_len is 1, a > user might expect their buffer to be filled with a null terminator, but > again, nothing will happen (even if strlen(res.path) is 0, which is unlikely > since new node's will have paths longer than "/"). > The name of the argument to zoo_create() is also a little misleading, as is > its description ("the maximum length of real path you would want") in > zookeeper.h, and the example usage in the Programmer's Guide: > {noformat} > int rc = zoo_create(zh,"/xyz","value", 5, &CREATE_ONLY, ZOO_EPHEMERAL, > buffer, sizeof(buffer)-1); > {noformat} > In fact this value should be the actual length of the buffer, including space > for the null terminator. If the user supplies a max_realpath_len of 10 and a > buffer of 11 bytes, and strlen(res.path) is 10, the code will truncate the > returned value to 9 bytes and put the null terminator in the second-last > byte, leaving the final byte of the buffer unused. > It would be better, I think, to rename the realpath and max_realpath_len > arguments to something like path_buffer and path_buffer_len, akin to > zoo_set(). The path_buffer_len would be treated as the full length of the > buffer (as the code does now, in fact, but the docs suggest otherwise). > The code in the synchronous completion could then be changed as per the > attached patch. > Since this would change, slightly, the behaviour or "contract" of the API, I > would be inclined to suggest waiting until 4.0.0 to implement this change. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.