PARQUET-825: Static analyzer findings (NPEs, resource leaks)

Some trivial code fixes based on findings on static code analyzer tools (Sonar, Fortify) @piyushnarang: Sorry, renaming the branch caused the closing of the original PR... Author: Gabor Szadovszky <[email protected]> Author: Gabor Szadovszky <[email protected]> Closes apache#399 from gszadovszky/PARQUET-825 and squashes the following commits: 68a4764 [Gabor Szadovszky] PARQUET-825 - Static analyzer findings (NPEs, resource leaks) a689c1c [Gabor Szadovszky] Code fixes related to null checks, exception handling and closing streams
7mming7 · Jan 26, 2017 · f68dbc3 · f68dbc3
1 parent 89e0607
commit f68dbc3
Show file tree

Hide file tree

Showing 8 changed files with 86 additions and 81 deletions.
diff --git a/parquet-column/src/main/java/org/apache/parquet/io/RecordReaderImplementation.java b/parquet-column/src/main/java/org/apache/parquet/io/RecordReaderImplementation.java
@@ -128,7 +128,8 @@ public boolean equals(Object obj) {
 //    }
 
     public boolean equals(Case other) {
-      return startLevel == other.startLevel
+      return other != null
+          && startLevel == other.startLevel
           && depth == other.depth
           && nextLevel == other.nextLevel
           && nextState == other.nextState

diff --git a/parquet-common/src/main/java/org/apache/parquet/glob/GlobNode.java b/parquet-common/src/main/java/org/apache/parquet/glob/GlobNode.java
@@ -58,7 +58,7 @@ public String get() {
     @Override
     public boolean equals(Object o) {
       if (this == o) return true;
-      return getClass() == o.getClass() && s.equals(((Atom) o).s);
+      return o != null && getClass() == o.getClass() && s.equals(((Atom) o).s);
     }
 
     @Override
@@ -97,7 +97,7 @@ public List<GlobNode> getChildren() {
     @Override
     public boolean equals(Object o) {
       if (this == o) return true;
-      return getClass() == o.getClass() && children.equals(((OneOf) o).children);
+      return o != null && getClass() == o.getClass() && children.equals(((OneOf) o).children);
     }
 
     @Override
@@ -136,7 +136,7 @@ public List<GlobNode> getChildren() {
     @Override
     public boolean equals(Object o) {
       if (this == o) return true;
-      return getClass() == o.getClass() && children.equals(((OneOf) o).children);
+      return o != null && getClass() == o.getClass() && children.equals(((OneOf) o).children);
     }
 
     @Override

diff --git a/...or/src/main/java/org/apache/parquet/encoding/bitpacking/ByteBasedBitPackingGenerator.java b/...or/src/main/java/org/apache/parquet/encoding/bitpacking/ByteBasedBitPackingGenerator.java
@@ -65,42 +65,42 @@ private static void generateScheme(boolean isLong, boolean msbFirst,
     if (!file.getParentFile().exists()) {
       file.getParentFile().mkdirs();
     }
-    FileWriter fw = new FileWriter(file);
-    fw.append("package org.apache.parquet.column.values.bitpacking;\n");
-    fw.append("import java.nio.ByteBuffer;\n");
-    fw.append("\n");
-    fw.append("/**\n");
-    if (msbFirst) {
-      fw.append(" * Packs from the Most Significant Bit first\n");
-    } else {
-      fw.append(" * Packs from the Least Significant Bit first\n");
-    }
-    fw.append(" * \n");
-    fw.append(" * @author automatically generated\n");
-    fw.append(" * @see ByteBasedBitPackingGenerator\n");
-    fw.append(" *\n");
-    fw.append(" */\n");
-    fw.append("public abstract class " + className + " {\n");
-    fw.append("\n");
-    fw.append("  private static final BytePacker" + nameSuffix + "[] packers = new BytePacker" + nameSuffix + "[" + (maxBits + 1) + "];\n");
-    fw.append("  static {\n");
-    for (int i = 0; i <= maxBits; i++) {
-      fw.append("    packers[" + i + "] = new Packer" + i + "();\n");
-    }
-    fw.append("  }\n");
-    fw.append("\n");
-    fw.append("  public static final BytePacker" + nameSuffix + "Factory factory = new BytePacker" + nameSuffix + "Factory() {\n");
-    fw.append("    public BytePacker" + nameSuffix + " newBytePacker" + nameSuffix + "(int bitWidth) {\n");
-    fw.append("      return packers[bitWidth];\n");
-    fw.append("    }\n");
-    fw.append("  };\n");
-    fw.append("\n");
-    for (int i = 0; i <= maxBits; i++) {
-      generateClass(fw, i, isLong, msbFirst);
+    try (FileWriter fw = new FileWriter(file)) {
+      fw.append("package org.apache.parquet.column.values.bitpacking;\n");
+      fw.append("import java.nio.ByteBuffer;\n");
+      fw.append("\n");
+      fw.append("/**\n");
+      if (msbFirst) {
+        fw.append(" * Packs from the Most Significant Bit first\n");
+      } else {
+        fw.append(" * Packs from the Least Significant Bit first\n");
+      }
+      fw.append(" * \n");
+      fw.append(" * @author automatically generated\n");
+      fw.append(" * @see ByteBasedBitPackingGenerator\n");
+      fw.append(" *\n");
+      fw.append(" */\n");
+      fw.append("public abstract class " + className + " {\n");
       fw.append("\n");
+      fw.append("  private static final BytePacker" + nameSuffix + "[] packers = new BytePacker" + nameSuffix + "[" + (maxBits + 1) + "];\n");
+      fw.append("  static {\n");
+      for (int i = 0; i <= maxBits; i++) {
+        fw.append("    packers[" + i + "] = new Packer" + i + "();\n");
+      }
+      fw.append("  }\n");
+      fw.append("\n");
+      fw.append("  public static final BytePacker" + nameSuffix + "Factory factory = new BytePacker" + nameSuffix + "Factory() {\n");
+      fw.append("    public BytePacker" + nameSuffix + " newBytePacker" + nameSuffix + "(int bitWidth) {\n");
+      fw.append("      return packers[bitWidth];\n");
+      fw.append("    }\n");
+      fw.append("  };\n");
+      fw.append("\n");
+      for (int i = 0; i <= maxBits; i++) {
+        generateClass(fw, i, isLong, msbFirst);
+        fw.append("\n");
+      }
+      fw.append("}\n");
     }
-    fw.append("}\n");
-    fw.close();
   }
 
   private static void generateClass(FileWriter fw, int bitWidth, boolean isLong, boolean msbFirst) throws IOException {

diff --git a/...tor/src/main/java/org/apache/parquet/encoding/bitpacking/IntBasedBitPackingGenerator.java b/...tor/src/main/java/org/apache/parquet/encoding/bitpacking/IntBasedBitPackingGenerator.java
@@ -56,45 +56,45 @@ private static void generateScheme(String className, boolean msbFirst, String ba
     if (!file.getParentFile().exists()) {
       file.getParentFile().mkdirs();
     }
-    FileWriter fw = new FileWriter(file);
-    fw.append("package org.apache.parquet.column.values.bitpacking;\n");
-    fw.append("\n");
-    fw.append("/**\n");
-    fw.append(" * Based on the original implementation at at https://github.com/lemire/JavaFastPFOR/blob/master/src/integercompression/BitPacking.java\n");
-    fw.append(" * Which is released under the\n");
-    fw.append(" * Apache License Version 2.0 http://www.apache.org/licenses/.\n");
-    fw.append(" * By Daniel Lemire, http://lemire.me/en/\n");
-    fw.append(" * \n");
-    fw.append(" * Scheme designed by D. Lemire\n");
-    if (msbFirst) {
-      fw.append(" * Adapted to pack from the Most Significant Bit first\n");
-    }
-    fw.append(" * \n");
-    fw.append(" * @author automatically generated\n");
-    fw.append(" * @see IntBasedBitPackingGenerator\n");
-    fw.append(" *\n");
-    fw.append(" */\n");
-    fw.append("abstract class " + className + " {\n");
-    fw.append("\n");
-    fw.append("  private static final IntPacker[] packers = new IntPacker[32];\n");
-    fw.append("  static {\n");
-    for (int i = 0; i < 32; i++) {
-      fw.append("    packers[" + i + "] = new Packer" + i + "();\n");
-    }
-    fw.append("  }\n");
-    fw.append("\n");
-    fw.append("  public static final IntPackerFactory factory = new IntPackerFactory() {\n");
-    fw.append("    public IntPacker newIntPacker(int bitWidth) {\n");
-    fw.append("      return packers[bitWidth];\n");
-    fw.append("    }\n");
-    fw.append("  };\n");
-    fw.append("\n");
-    for (int i = 0; i < 32; i++) {
-      generateClass(fw, i, msbFirst);
+    try (FileWriter fw = new FileWriter(file)) {
+      fw.append("package org.apache.parquet.column.values.bitpacking;\n");
+      fw.append("\n");
+      fw.append("/**\n");
+      fw.append(" * Based on the original implementation at at https://github.com/lemire/JavaFastPFOR/blob/master/src/integercompression/BitPacking.java\n");
+      fw.append(" * Which is released under the\n");
+      fw.append(" * Apache License Version 2.0 http://www.apache.org/licenses/.\n");
+      fw.append(" * By Daniel Lemire, http://lemire.me/en/\n");
+      fw.append(" * \n");
+      fw.append(" * Scheme designed by D. Lemire\n");
+      if (msbFirst) {
+        fw.append(" * Adapted to pack from the Most Significant Bit first\n");
+      }
+      fw.append(" * \n");
+      fw.append(" * @author automatically generated\n");
+      fw.append(" * @see IntBasedBitPackingGenerator\n");
+      fw.append(" *\n");
+      fw.append(" */\n");
+      fw.append("abstract class " + className + " {\n");
       fw.append("\n");
+      fw.append("  private static final IntPacker[] packers = new IntPacker[32];\n");
+      fw.append("  static {\n");
+      for (int i = 0; i < 32; i++) {
+        fw.append("    packers[" + i + "] = new Packer" + i + "();\n");
+      }
+      fw.append("  }\n");
+      fw.append("\n");
+      fw.append("  public static final IntPackerFactory factory = new IntPackerFactory() {\n");
+      fw.append("    public IntPacker newIntPacker(int bitWidth) {\n");
+      fw.append("      return packers[bitWidth];\n");
+      fw.append("    }\n");
+      fw.append("  };\n");
+      fw.append("\n");
+      for (int i = 0; i < 32; i++) {
+        generateClass(fw, i, msbFirst);
+        fw.append("\n");
+      }
+      fw.append("}\n");
     }
-    fw.append("}\n");
-    fw.close();
   }
 
   private static void generateClass(FileWriter fw, int bitWidth, boolean msbFirst) throws IOException {

diff --git a/parquet-generator/src/main/java/org/apache/parquet/version/VersionGenerator.java b/parquet-generator/src/main/java/org/apache/parquet/version/VersionGenerator.java
@@ -50,7 +50,11 @@ public void run() throws IOException {
     	throw new IOException("/parquet-version.properties not found");
     }
     Properties props = new Properties();
-    props.load(in);
+    try {
+      props.load(in);
+    } finally {
+      in.close();
+    }
 
     add("package org.apache.parquet;\n" +
         "\n" +

diff --git a/parquet-tools/src/main/java/org/apache/parquet/tools/command/DumpCommand.java b/parquet-tools/src/main/java/org/apache/parquet/tools/command/DumpCommand.java
@@ -331,7 +331,7 @@ public static String binaryToString(Binary value) {
         try {
             CharBuffer buffer = UTF8_DECODER.decode(value.toByteBuffer());
             return buffer.toString();
-        } catch (Throwable th) {
+        } catch (Exception ex) {
         }
 
         return "<bytes...>";

diff --git a/parquet-tools/src/main/java/org/apache/parquet/tools/command/Registry.java b/parquet-tools/src/main/java/org/apache/parquet/tools/command/Registry.java
@@ -39,7 +39,7 @@ public static Map<String,Command> allCommands() {
     for (Map.Entry<String,Class<? extends Command>> entry : registry.entrySet()) {
       try {
         results.put(entry.getKey(), entry.getValue().newInstance());
-      } catch (Throwable th) {
+      } catch (Exception ex) {
       }
     }
 
@@ -54,7 +54,7 @@ public static Command getCommandByName(String name) {
 
     try {
       return clazz.newInstance();
-    } catch (Throwable th) {
+    } catch (Exception ex) {
       return null;
     }
   }

diff --git a/parquet-tools/src/main/java/org/apache/parquet/tools/util/PrettyPrintWriter.java b/parquet-tools/src/main/java/org/apache/parquet/tools/util/PrettyPrintWriter.java
@@ -79,7 +79,7 @@ public enum WhiteSpaceHandler { ELIMINATE_NEWLINES, COLLAPSE_WHITESPACE }
     if (columns != null && !columns.isEmpty()) {
       try {
         consoleWidth = Integer.parseInt(columns);
-      } catch (Throwable th) {
+      } catch (Exception ex) {
       }
     }
 
@@ -88,7 +88,7 @@ public enum WhiteSpaceHandler { ELIMINATE_NEWLINES, COLLAPSE_WHITESPACE }
       try {
         numColors = Integer.parseInt(colors);
         if (numColors < 0) numColors = 0;
-      } catch (Throwable th) {
+      } catch (Exception exa) {
       }
     }