zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rake...@apache.org
Subject zookeeper git commit: ZOOKEEPER-2733: Cleanup findbug warnings in branch-3.4: Dodgy code Warnings
Date Wed, 24 May 2017 14:55:25 GMT
Repository: zookeeper
Updated Branches:
  refs/heads/branch-3.4 6522d3f4d -> a378ea163


ZOOKEEPER-2733: Cleanup findbug warnings in branch-3.4: Dodgy code Warnings

Author: Abraham Fine <afine@apache.org>

Reviewers: Rakesh Radhakrishnan <rakeshr@apache.org>

Closes #236 from afine/ZOOKEEPER-2733


Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/a378ea16
Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/a378ea16
Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/a378ea16

Branch: refs/heads/branch-3.4
Commit: a378ea163b7110fa6f8aaf5fb07986d7901b1950
Parents: 6522d3f
Author: Abraham Fine <afine@apache.org>
Authored: Wed May 24 02:25:24 2017 -0700
Committer: Rakesh Radhakrishnan <rakeshr@apache.org>
Committed: Wed May 24 02:25:24 2017 -0700

----------------------------------------------------------------------
 .../zookeeper/server/PrepRequestProcessor.java  |  9 ++++++---
 .../apache/zookeeper/server/PurgeTxnLog.java    | 15 +++++++++++----
 .../zookeeper/server/SyncRequestProcessor.java  |  2 +-
 .../server/persistence/FileTxnLog.java          |  2 --
 .../zookeeper/server/quorum/Follower.java       |  2 ++
 .../zookeeper/server/quorum/LearnerHandler.java |  2 +-
 .../zookeeper/server/quorum/Observer.java       |  2 ++
 .../quorum/auth/SaslQuorumAuthLearner.java      | 10 ++--------
 .../quorum/auth/SaslQuorumAuthServer.java       |  2 +-
 .../zookeeper/server/upgrade/DataTreeV1.java    | 17 -----------------
 .../zookeeper/server/upgrade/UpgradeMain.java   | 20 +++++++++++---------
 src/java/test/config/findbugsExcludeFile.xml    |  7 ++++++-
 12 files changed, 43 insertions(+), 47 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java b/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
index 58497b7..825c22a 100644
--- a/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
+++ b/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
@@ -505,6 +505,8 @@ public class PrepRequestProcessor extends ZooKeeperCriticalThread implements
                 version = currentVersion + 1;
                 request.txn = new CheckVersionTxn(path, version);
                 break;
+            default:
+                LOG.error("Invalid OpCode: {} received by PrepRequestProcessor", type);
         }
     }
 
@@ -588,9 +590,7 @@ public class PrepRequestProcessor extends ZooKeeperCriticalThread implements
                         try {
                             pRequest2Txn(op.getType(), zxid, request, subrequest, false);
                         } catch (KeeperException e) {
-                            if (ke == null) {
-                                ke = e;
-                            }
+                            ke = e;
                             request.hdr.setType(OpCode.error);
                             request.txn = new ErrorTxn(e.code().intValue());
                             LOG.info("Got user-level KeeperException when processing "
@@ -641,6 +641,9 @@ public class PrepRequestProcessor extends ZooKeeperCriticalThread implements
                 zks.sessionTracker.checkSession(request.sessionId,
                         request.getOwner());
                 break;
+            default:
+                LOG.warn("unknown type " + request.type);
+                break;
             }
         } catch (KeeperException e) {
             if (request.hdr != null) {

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/PurgeTxnLog.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/PurgeTxnLog.java b/src/java/main/org/apache/zookeeper/server/PurgeTxnLog.java
index d4effaf..11112a4 100644
--- a/src/java/main/org/apache/zookeeper/server/PurgeTxnLog.java
+++ b/src/java/main/org/apache/zookeeper/server/PurgeTxnLog.java
@@ -133,11 +133,18 @@ public class PurgeTxnLog {
             }
         }
         // add all non-excluded log files
-        List<File> files = new ArrayList<File>(Arrays.asList(txnLog
-                .getDataDir().listFiles(new MyFileFilter(PREFIX_LOG))));
+        List<File> files = new ArrayList<File>();
+        File[] fileArray = txnLog.getDataDir().listFiles(new MyFileFilter(PREFIX_LOG));
+        if (fileArray != null) {
+            files.addAll(Arrays.asList(fileArray));
+        }
+
         // add all non-excluded snapshot files to the deletion list
-        files.addAll(Arrays.asList(txnLog.getSnapDir().listFiles(
-                new MyFileFilter(PREFIX_SNAPSHOT))));
+        fileArray = txnLog.getSnapDir().listFiles(new MyFileFilter(PREFIX_SNAPSHOT));
+        if (fileArray != null) {
+            files.addAll(Arrays.asList(fileArray));
+        }
+
         // remove the old files
         for(File f: files)
         {

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java b/src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java
index f622b24..317b4ac 100644
--- a/src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java
+++ b/src/java/main/org/apache/zookeeper/server/SyncRequestProcessor.java
@@ -140,7 +140,7 @@ public class SyncRequestProcessor extends ZooKeeperCriticalThread implements
Req
                     if (zks.getZKDatabase().append(si)) {
                         logCount++;
                         if (logCount > (snapCount / 2 + randRoll)) {
-                            randRoll = r.nextInt(snapCount/2);
+                            setRandRoll(r.nextInt(snapCount/2));
                             // roll the log
                             zks.getZKDatabase().rollLog();
                             // take a snapshot

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java b/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java
index b1682c3..690cfca 100644
--- a/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java
+++ b/src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java
@@ -637,8 +637,6 @@ public class FileTxnLog implements TxnLog {
                 crc.update(bytes, 0, bytes.length);
                 if (crcValue != crc.getValue())
                     throw new IOException(CRC_ERROR);
-                if (bytes == null || bytes.length == 0)
-                    return false;
                 hdr = new TxnHeader();
                 record = SerializeUtils.deserializeTxn(bytes, hdr);
             } catch (EOFException e) {

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/quorum/Follower.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/quorum/Follower.java b/src/java/main/org/apache/zookeeper/server/quorum/Follower.java
index e439aaa..27c3375 100644
--- a/src/java/main/org/apache/zookeeper/server/quorum/Follower.java
+++ b/src/java/main/org/apache/zookeeper/server/quorum/Follower.java
@@ -136,6 +136,8 @@ public class Follower extends Learner{
         case Leader.SYNC:
             fzk.sync();
             break;
+        default:
+            LOG.error("Invalid packet type: {} received by Observer", qp.getType());
         }
     }
 

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java b/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java
index 51ed7e7..4820490 100644
--- a/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java
+++ b/src/java/main/org/apache/zookeeper/server/quorum/LearnerHandler.java
@@ -272,7 +272,7 @@ public class LearnerHandler extends ZooKeeperThread {
             type = "PROPOSAL";
             TxnHeader hdr = new TxnHeader();
             try {
-                txn = SerializeUtils.deserializeTxn(p.getData(), hdr);
+                SerializeUtils.deserializeTxn(p.getData(), hdr);
                 // mess = "transaction: " + txn.toString();
             } catch (IOException e) {
                 LOG.warn("Unexpected exception",e);

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/quorum/Observer.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/quorum/Observer.java b/src/java/main/org/apache/zookeeper/server/quorum/Observer.java
index 53f516f..ded4a24 100644
--- a/src/java/main/org/apache/zookeeper/server/quorum/Observer.java
+++ b/src/java/main/org/apache/zookeeper/server/quorum/Observer.java
@@ -125,6 +125,8 @@ public class Observer extends Learner{
             ObserverZooKeeperServer obs = (ObserverZooKeeperServer)zk;
             obs.commitRequest(request);            
             break;
+        default:
+            LOG.error("Invalid packet type: {} received by Observer", qp.getType());
         }
     }
 

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java
b/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java
index 9a38bb8..8c76d3e 100644
--- a/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java
+++ b/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthLearner.java
@@ -173,14 +173,8 @@ public class SaslQuorumAuthLearner implements QuorumAuthLearner {
         BufferedOutputStream bufferedOutput = new BufferedOutputStream(dout);
         BinaryOutputArchive boa = BinaryOutputArchive
                 .getArchive(bufferedOutput);
-        if (response == null) {
-            authPacket = QuorumAuth.createPacket(
-                    QuorumAuth.Status.IN_PROGRESS, response);
-        } else {
-            authPacket = QuorumAuth.createPacket(
-                    QuorumAuth.Status.IN_PROGRESS, response);
-        }
-
+        authPacket = QuorumAuth.createPacket(
+                QuorumAuth.Status.IN_PROGRESS, response);
         boa.writeRecord(authPacket, QuorumAuth.QUORUM_AUTH_MESSAGE_TAG);
         bufferedOutput.flush();
     }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java
b/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java
index a64513a..1aa2b32 100644
--- a/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java
+++ b/src/java/main/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java
@@ -169,7 +169,7 @@ public class SaslQuorumAuthServer implements QuorumAuthServer {
         QuorumAuthPacket authPacket;
         if (challenge == null && s != QuorumAuth.Status.SUCCESS) {
             authPacket = QuorumAuth.createPacket(
-                    QuorumAuth.Status.IN_PROGRESS, challenge);
+                    QuorumAuth.Status.IN_PROGRESS, null);
         } else {
             authPacket = QuorumAuth.createPacket(s, challenge);
         }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/upgrade/DataTreeV1.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/upgrade/DataTreeV1.java b/src/java/main/org/apache/zookeeper/server/upgrade/DataTreeV1.java
index e3d0633..678f4c6 100644
--- a/src/java/main/org/apache/zookeeper/server/upgrade/DataTreeV1.java
+++ b/src/java/main/org/apache/zookeeper/server/upgrade/DataTreeV1.java
@@ -347,14 +347,6 @@ public class DataTreeV1 {
 
         public long zxid;
 
-        public int err;
-
-        public int type;
-
-        public String path;
-
-        public Stat stat;
-
         /**
          * Equality is defined as the clientId and the cxid being the same. This
          * allows us to use hash tables to track completion of transactions.
@@ -394,8 +386,6 @@ public class DataTreeV1 {
             rc.clientId = header.getClientId();
             rc.cxid = header.getCxid();
             rc.zxid = header.getZxid();
-            rc.type = header.getType();
-            rc.err = 0;
             if (rc.zxid > lastProcessedZxid) {
                 lastProcessedZxid = rc.zxid;
             }
@@ -406,7 +396,6 @@ public class DataTreeV1 {
                 createNode(createTxn.getPath(), createTxn.getData(), createTxn
                         .getAcl(), createTxn.getEphemeral() ? header
                         .getClientId() : 0, header.getZxid(), header.getTime());
-                rc.path = createTxn.getPath();
                 break;
             case OpCode.delete:
                 DeleteTxn deleteTxn = (DeleteTxn) txn;
@@ -416,22 +405,16 @@ public class DataTreeV1 {
             case OpCode.setData:
                 SetDataTxn setDataTxn = (SetDataTxn) txn;
                 debug = "Set data for  transaction for " + setDataTxn.getPath();
-                rc.stat = setData(setDataTxn.getPath(), setDataTxn.getData(),
-                        setDataTxn.getVersion(), header.getZxid(), header
-                                .getTime());
                 break;
             case OpCode.setACL:
                 SetACLTxn setACLTxn = (SetACLTxn) txn;
                 debug = "Set ACL for  transaction for " + setACLTxn.getPath();
-                rc.stat = setACL(setACLTxn.getPath(), setACLTxn.getAcl(),
-                        setACLTxn.getVersion());
                 break;
             case OpCode.closeSession:
                 killSession(header.getClientId());
                 break;
             case OpCode.error:
                 ErrorTxn errTxn = (ErrorTxn) txn;
-                rc.err = errTxn.getErr();
                 break;
             }
         } catch (KeeperException e) {

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/main/org/apache/zookeeper/server/upgrade/UpgradeMain.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/upgrade/UpgradeMain.java b/src/java/main/org/apache/zookeeper/server/upgrade/UpgradeMain.java
index 779c481..6648a12 100644
--- a/src/java/main/org/apache/zookeeper/server/upgrade/UpgradeMain.java
+++ b/src/java/main/org/apache/zookeeper/server/upgrade/UpgradeMain.java
@@ -114,15 +114,17 @@ public class UpgradeMain {
      */
     void copyFiles(File srcDir, File dstDir, String filter) throws IOException {
         File[] list = srcDir.listFiles();
-        for (File file: list) {
-            String name = file.getName();
-            if (name.startsWith(filter)) {
-                // we need to copy this file
-                File dest = new File(dstDir, name);
-                LOG.info("Renaming " + file + " to " + dest);
-                if (!file.renameTo(dest)) {
-                    throw new IOException("Unable to rename " 
-                            + file + " to " +  dest);
+        if (list != null) {
+            for (File file: list) {
+                String name = file.getName();
+                if (name.startsWith(filter)) {
+                    // we need to copy this file
+                    File dest = new File(dstDir, name);
+                    LOG.info("Renaming " + file + " to " + dest);
+                    if (!file.renameTo(dest)) {
+                        throw new IOException("Unable to rename "
+                                + file + " to " +  dest);
+                    }
                 }
             }
         }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/a378ea16/src/java/test/config/findbugsExcludeFile.xml
----------------------------------------------------------------------
diff --git a/src/java/test/config/findbugsExcludeFile.xml b/src/java/test/config/findbugsExcludeFile.xml
index 7c6c532..de8e078 100644
--- a/src/java/test/config/findbugsExcludeFile.xml
+++ b/src/java/test/config/findbugsExcludeFile.xml
@@ -96,7 +96,12 @@
   <Match>
     <Class name="org.apache.zookeeper.server.quorum.AuthFastLeaderElection$Messenger$WorkerSender"/>
     <Method name="process"/>
-    <Bug code="RV"/>
+    <Bug code="RV,SF"/>
+  </Match>
+  <Match>
+    <Class name="org.apache.zookeeper.server.quorum.AuthFastLeaderElection$Messenger$WorkerReceiver"/>
+    <Method name="run"/>
+    <Bug code="SF"/>
   </Match>
 
   <!-- these are old classes just for upgrading and should go away -->


Mime
View raw message