You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2020/09/30 21:07:26 UTC

[GitHub] [zookeeper] js-musedev opened a new pull request #1470: fix some bugs found by muse.dev

js-musedev opened a new pull request #1470:
URL: https://github.com/apache/zookeeper/pull/1470


   Fixed a resource leak, a potential null pointer exception, and replaced the cleartext socket creation with an encrypted one.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] hanm commented on pull request #1470: fix some bugs found by muse.dev

Posted by GitBox <gi...@apache.org>.
hanm commented on pull request #1470:
URL: https://github.com/apache/zookeeper/pull/1470#issuecomment-701852453


   please take a look at https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute - specifically:
   * please create a jira first.
   * please make sure run all tests locally and make sure they pass and make sure CI is green (this PR currently fails quite a few tests).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] js-musedev commented on a change in pull request #1470: fix some bugs found by muse.dev

Posted by GitBox <gi...@apache.org>.
js-musedev commented on a change in pull request #1470:
URL: https://github.com/apache/zookeeper/pull/1470#discussion_r497811125



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java
##########
@@ -1716,6 +1720,7 @@ public void revalidateSession(QuorumPacket qp, LearnerHandler learnerHandler) th
         DataInputStream dis = new DataInputStream(bis);
         long id = dis.readLong();
         int to = dis.readInt();
+        dis.close();

Review comment:
       Yep. Corrected in new commit.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [zookeeper] ctubbsii commented on a change in pull request #1470: fix some bugs found by muse.dev

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1470:
URL: https://github.com/apache/zookeeper/pull/1470#discussion_r497804158



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java
##########
@@ -1716,6 +1720,7 @@ public void revalidateSession(QuorumPacket qp, LearnerHandler learnerHandler) th
         DataInputStream dis = new DataInputStream(bis);
         long id = dis.readLong();
         int to = dis.readInt();
+        dis.close();

Review comment:
       Would this be better as:
   ```suggestion
           long id;
           int to;
           try (DataInputStream dis = new DataInputStream(bis)) {
               id = dis.readLong();
               to = dis.readInt();
           }
   ```
   The `bis` variable could also be included in the same try-with-resources block (but GitHub review interface won't let me select that line to include it in the suggested changes).




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org