You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by phunt <gi...@git.apache.org> on 2018/05/21 18:06:01 UTC
[GitHub] zookeeper issue #525: [ZOOKEEPER-3009] Potential NPE in NIOServerCnxnFactory
Github user phunt commented on the issue:
https://github.com/apache/zookeeper/pull/525
These are useful fixes. However one of the burdens of recommending such a fix is to also prove that the fix really addresses the issue, rather than just moving the problem somewhere else. In particular it would be good if the description of this fix (and any such fix) included details on whether all of the callers to addCnxn handle the new error handling code properly. In this case throwing the IOException in addCnxn. "some of their callers have !=null check but some do not have" One of the things any committer will need to do is also verify, and having this thought out ahead of time will help.
Another thing - are the null checks in the caller code still necessary? Should they be removed? If yes any such patch should include changes there as well.
---