You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/11/14 10:57:52 UTC

[tomcat] 05/05: Fix some of the SpotBugs warnings in o.a.tomcat

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 547f25c49ecbd179a561de4e81cdc29ee8d1c6ed
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Nov 14 10:48:15 2019 +0000

    Fix some of the SpotBugs warnings in o.a.tomcat
---
 .../apache/tomcat/buildutil/translate/Utils.java   |  6 ++-
 res/findbugs/filter-false-positives.xml            | 61 ++++++++++++++++++++--
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/java/org/apache/tomcat/buildutil/translate/Utils.java b/java/org/apache/tomcat/buildutil/translate/Utils.java
index 8b69c79..5eb015f 100644
--- a/java/org/apache/tomcat/buildutil/translate/Utils.java
+++ b/java/org/apache/tomcat/buildutil/translate/Utils.java
@@ -78,7 +78,11 @@ public class Utils {
 
 
     static void processDirectory(File root, File dir, Map<String,Properties> translations) throws IOException {
-        for (File f : dir.listFiles()) {
+        File[] files = dir.listFiles();
+        if (files == null) {
+            throw new IllegalArgumentException("Not a directory [" + dir.getAbsolutePath() + "]");
+        }
+        for (File f : files) {
             if (f.isDirectory()) {
                 processDirectory(root, f, translations);
             } else if (f.isFile()) {
diff --git a/res/findbugs/filter-false-positives.xml b/res/findbugs/filter-false-positives.xml
index fd1b463..c691b6c 100644
--- a/res/findbugs/filter-false-positives.xml
+++ b/res/findbugs/filter-false-positives.xml
@@ -1011,6 +1011,14 @@
     <Bug code="Nm" />
   </Match>
   <Match>
+    <!-- Utility classes used to import/export l10n strings -->
+    <!-- This code does not need to be robust -->
+    <Or>
+      <Class name="org.apache.tomcat.buildutil.translate.Export"/>
+      <Class name="org.apache.tomcat.buildutil.translate.Import"/>
+    </Or>
+  </Match>
+  <Match>
     <!-- Return value is never used -->
     <Class name="org.apache.tomcat.dbcp.dbcp2.DelegatingConnection" />
     <Method name="prepareStatement" />
@@ -1063,6 +1071,11 @@
     <Bug pattern="NP_EQUALS_SHOULD_HANDLE_NULL_ARGUMENT" />
   </Match>
   <Match>
+    <!-- Natural ordering behaviour is docuemented in Javadoc -->
+    <Class name="org.apache.tomcat.dbcp.pool2.impl.DefaultPooledObject" />
+    <Bug pattern="EQ_COMPARETO_USE_OBJECT_EQUALS" />
+  </Match>
+  <Match>
     <!-- Increment is in sync block so it is safe. Volatile is used so reading
          thread sees latest value.  -->
     <Class name="org.apache.tomcat.dbcp.pool2.impl.DefaultPooledObject" />
@@ -1070,6 +1083,15 @@
     <Bug pattern="VO_VOLATILE_INCREMENT" />
   </Match>
   <Match>
+    <!-- Fields do not need to be sync'd for toString() -->
+    <Class name="org.apache.tomcat.dbcp.pool2.impl.SoftReferenceObjectPool" />
+    <Or>
+      <Field name="createCount"/>
+      <Field name="numActive"/>
+    </Or>
+    <Bug pattern="IS2_INCONSISTENT_SYNC" />
+  </Match>
+  <Match>
     <!-- Return value is ignored but a null result will trigger an exception -->
     <Class name="org.apache.tomcat.jdbc.pool.ConnectionPool$ConnectionFuture" />
     <Method name="get" />
@@ -1110,6 +1132,15 @@
     <Bug code="VO" />
   </Match>
   <Match>
+    <!-- Fields are used by native code. Tomcat doesn't use them but they are
+         part of the public API. -->
+    <Or>
+      <Class name="org.apache.tomcat.jni.FileInfo" />
+      <Class name="org.apache.tomcat.jni.Sockaddr" />
+    </Or>
+    <Bug pattern="UUF_UNUSED_PUBLIC_OR_PROTECTED_FIELD" />
+  </Match>
+  <Match>
     <Class name="org.apache.tomcat.util.IntrospectionUtils" />
     <Method name="findMethod"/>
     <Bug code="NP" />
@@ -1130,14 +1161,19 @@
     <Bug code="SF" />
   </Match>
   <Match>
-    <!-- Returning null here is fine -->
-    <Class name="org.apache.tomcat.util.buf.ByteChunk"/>
-    <Method name="toString"/>
-    <Bug code="NP" />
+    <!-- Handled by abstract base class -->
+    <Or>
+      <Class name="org.apache.tomcat.util.buf.ByteChunk"/>
+      <Class name="org.apache.tomcat.util.buf.CharChunk"/>
+    </Or>
+    <Bug pattern="HE_EQUALS_NO_HASHCODE" />
   </Match>
   <Match>
     <!-- Returning null here is fine -->
-    <Class name="org.apache.tomcat.util.buf.CharChunk"/>
+    <Or>
+      <Class name="org.apache.tomcat.util.buf.ByteChunk"/>
+      <Class name="org.apache.tomcat.util.buf.CharChunk"/>
+    </Or>
     <Method name="toString"/>
     <Bug code="NP" />
   </Match>
@@ -1154,6 +1190,15 @@
     <Bug code="ST" />
   </Match>
   <Match>
+    <!-- Array is only ever updated as a whole, not element by element -->
+    <Class name="org.apache.tomcat.util.buf.StringCache"/>
+    <Or>
+      <Field name="bcCache"/>
+      <Field name="ccCache"/>
+    </Or>
+    <Bug pattern="VO_VOLATILE_REFERENCE_TO_ARRAY"/>
+  </Match>
+  <Match>
     <!-- mb.toString() can be null because
     o.a.t.util.buf.MessageBytes.toString() can return NULL -->
     <Class name="org.apache.tomcat.util.buf.UDecoder"/>
@@ -1182,6 +1227,12 @@
     <Bug code="ES" />
   </Match>
   <Match>
+    <!-- Write to static field is intentional -->
+    <Class name="org.apache.tomcat.util.digester.Digester"/>
+    <Method name="&lt;init&gt;"/>
+    <Bug pattern="ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD" />
+  </Match>
+  <Match>
     <!-- Fall-through expected -->
     <Class name="org.apache.tomcat.util.http.LegacyCookieProcessor" />
     <Method name="processCookieHeader"/>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org