From 58016fd78daf5181077b076669b355d2fd479d7b Mon Sep 17 00:00:00 2001 From: "Taro L. Saito" Date: Fri, 15 May 2015 12:21:12 +0900 Subject: [PATCH] Remove findbugs reported problems --- src/main/java/org/xerial/snappy/Snappy.java | 18 +++--- .../java/org/xerial/snappy/SnappyCodec.java | 10 +++- .../java/org/xerial/snappy/SnappyLoader.java | 57 ++++++++++++------- .../org/xerial/snappy/SnappyOutputStream.java | 2 +- .../snappy/buffer/CachedBufferAllocator.java | 16 +++++- .../xerial/snappy/SnappyOutputStreamTest.java | 2 +- 6 files changed, 69 insertions(+), 36 deletions(-) diff --git a/src/main/java/org/xerial/snappy/Snappy.java b/src/main/java/org/xerial/snappy/Snappy.java index 054c2e3..f96a06e 100755 --- a/src/main/java/org/xerial/snappy/Snappy.java +++ b/src/main/java/org/xerial/snappy/Snappy.java @@ -476,7 +476,7 @@ public class Snappy throws IOException { byte[] result = new byte[Snappy.uncompressedLength(input)]; - int byteSize = Snappy.uncompress(input, 0, input.length, result, 0); + Snappy.uncompress(input, 0, input.length, result, 0); return result; } @@ -569,7 +569,7 @@ public class Snappy { int uncompressedLength = Snappy.uncompressedLength(input, offset, length); char[] result = new char[uncompressedLength / 2]; - int byteSize = impl.rawUncompress(input, offset, length, result, 0); + impl.rawUncompress(input, offset, length, result, 0); return result; } @@ -585,7 +585,7 @@ public class Snappy { int uncompressedLength = Snappy.uncompressedLength(input, 0, input.length); double[] result = new double[uncompressedLength / 8]; - int byteSize = impl.rawUncompress(input, 0, input.length, result, 0); + impl.rawUncompress(input, 0, input.length, result, 0); return result; } @@ -687,7 +687,7 @@ public class Snappy { int uncompressedLength = Snappy.uncompressedLength(input, offset, length); float[] result = new float[uncompressedLength / 4]; - int byteSize = impl.rawUncompress(input, offset, length, result, 0); + impl.rawUncompress(input, offset, length, result, 0); return result; } @@ -718,7 +718,7 @@ public class Snappy { int uncompressedLength = Snappy.uncompressedLength(input, offset, length); int[] result = new int[uncompressedLength / 4]; - int byteSize = impl.rawUncompress(input, offset, length, result, 0); + impl.rawUncompress(input, offset, length, result, 0); return result; } @@ -749,7 +749,7 @@ public class Snappy { int uncompressedLength = Snappy.uncompressedLength(input, offset, length); long[] result = new long[uncompressedLength / 8]; - int byteSize = impl.rawUncompress(input, offset, length, result, 0); + impl.rawUncompress(input, offset, length, result, 0); return result; } @@ -780,7 +780,7 @@ public class Snappy { int uncompressedLength = Snappy.uncompressedLength(input, offset, length); short[] result = new short[uncompressedLength / 2]; - int byteSize = impl.rawUncompress(input, offset, length, result, 0); + impl.rawUncompress(input, offset, length, result, 0); return result; } @@ -838,7 +838,7 @@ public class Snappy UnsupportedEncodingException { byte[] uncompressed = new byte[uncompressedLength(input, offset, length)]; - int compressedSize = uncompress(input, offset, length, uncompressed, 0); + uncompress(input, offset, length, uncompressed, 0); return new String(uncompressed, encoding); } @@ -858,7 +858,7 @@ public class Snappy UnsupportedEncodingException { byte[] uncompressed = new byte[uncompressedLength(input, offset, length)]; - int compressedSize = uncompress(input, offset, length, uncompressed, 0); + uncompress(input, offset, length, uncompressed, 0); return new String(uncompressed, encoding); } diff --git a/src/main/java/org/xerial/snappy/SnappyCodec.java b/src/main/java/org/xerial/snappy/SnappyCodec.java index b5e1791..588236e 100755 --- a/src/main/java/org/xerial/snappy/SnappyCodec.java +++ b/src/main/java/org/xerial/snappy/SnappyCodec.java @@ -48,7 +48,7 @@ import java.util.Arrays; */ public class SnappyCodec { - public static final byte[] MAGIC_HEADER = new byte[] {-126, 'S', 'N', 'A', 'P', 'P', 'Y', 0}; + static final byte[] MAGIC_HEADER = new byte[] {-126, 'S', 'N', 'A', 'P', 'P', 'Y', 0}; public static final int MAGIC_LEN = MAGIC_HEADER.length; public static final int HEADER_SIZE = MAGIC_LEN + 8; public static final int MAGIC_HEADER_HEAD = SnappyOutputStream.readInt(MAGIC_HEADER, 0); @@ -59,6 +59,7 @@ public class SnappyCodec public static final int DEFAULT_VERSION = 1; public static final int MINIMUM_COMPATIBLE_VERSION = 1; + public static final SnappyCodec currentHeader = new SnappyCodec(MAGIC_HEADER, DEFAULT_VERSION, MINIMUM_COMPATIBLE_VERSION); public final byte[] magic; public final int version; @@ -85,6 +86,11 @@ public class SnappyCodec headerArray = header.toByteArray(); } + public static byte[] getMagicHeader() + { + return MAGIC_HEADER.clone(); + } + @Override public String toString() { @@ -124,7 +130,5 @@ public class SnappyCodec int compatibleVersion = d.readInt(); return new SnappyCodec(magic, version, compatibleVersion); } - - public static SnappyCodec currentHeader = new SnappyCodec(MAGIC_HEADER, DEFAULT_VERSION, MINIMUM_COMPATIBLE_VERSION); } diff --git a/src/main/java/org/xerial/snappy/SnappyLoader.java b/src/main/java/org/xerial/snappy/SnappyLoader.java index 2cff1a6..11f40a4 100755 --- a/src/main/java/org/xerial/snappy/SnappyLoader.java +++ b/src/main/java/org/xerial/snappy/SnappyLoader.java @@ -87,7 +87,10 @@ public class SnappyLoader static void cleanUpExtractedNativeLib() { if (nativeLibFile != null && nativeLibFile.exists()) { - nativeLibFile.delete(); + boolean deleted = nativeLibFile.delete(); + if (!deleted) { + // Deleting native lib has failed, but it's not serious so simply ignore it here + } } } @@ -217,37 +220,50 @@ public class SnappyLoader try { // Extract a native library file into the target directory - InputStream reader = SnappyLoader.class.getResourceAsStream(nativeLibraryFilePath); - FileOutputStream writer = new FileOutputStream(extractedLibFile); + InputStream reader = null; + FileOutputStream writer = null; try { - byte[] buffer = new byte[8192]; - int bytesRead = 0; - while ((bytesRead = reader.read(buffer)) != -1) { - writer.write(buffer, 0, bytesRead); + reader = SnappyLoader.class.getResourceAsStream(nativeLibraryFilePath); + try { + writer = new FileOutputStream(extractedLibFile); + + byte[] buffer = new byte[8192]; + int bytesRead = 0; + while ((bytesRead = reader.read(buffer)) != -1) { + writer.write(buffer, 0, bytesRead); + } + } + finally { + if (writer != null) { + writer.close(); + } } } finally { - // Delete the extracted lib file on JVM exit. - extractedLibFile.deleteOnExit(); - - if (writer != null) { - writer.close(); - } if (reader != null) { reader.close(); } + + // Delete the extracted lib file on JVM exit. + extractedLibFile.deleteOnExit(); } // Set executable (x) flag to enable Java to load the native library - extractedLibFile.setReadable(true); - extractedLibFile.setWritable(true, true); - extractedLibFile.setExecutable(true); + boolean success = extractedLibFile.setReadable(true) && + extractedLibFile.setWritable(true, true) && + extractedLibFile.setExecutable(true); + if (!success) { + // Setting file flag may fail, but in this case another error will be thrown in later phase + } // Check whether the contents are properly copied from the resource folder { - InputStream nativeIn = SnappyLoader.class.getResourceAsStream(nativeLibraryFilePath); - InputStream extractedLibIn = new FileInputStream(extractedLibFile); + InputStream nativeIn = null; + InputStream extractedLibIn = null; try { + nativeIn = SnappyLoader.class.getResourceAsStream(nativeLibraryFilePath); + extractedLibIn = new FileInputStream(extractedLibFile); + if (!contentsEquals(nativeIn, extractedLibIn)) { throw new SnappyError(SnappyErrorCode.FAILED_TO_LOAD_NATIVE_LIBRARY, String.format("Failed to write a native library file at %s", extractedLibFile)); } @@ -318,7 +334,10 @@ public class SnappyLoader // Temporary folder for the native lib. Use the value of org.xerial.snappy.tempdir or java.io.tmpdir File tempFolder = new File(System.getProperty(KEY_SNAPPY_TEMPDIR, System.getProperty("java.io.tmpdir"))); if (!tempFolder.exists()) { - tempFolder.mkdir(); + boolean created = tempFolder.mkdirs(); + if (!created) { + // if created == false, it will fail eventually in the later part + } } // Extract and load a native library inside the jar file diff --git a/src/main/java/org/xerial/snappy/SnappyOutputStream.java b/src/main/java/org/xerial/snappy/SnappyOutputStream.java index b25b0b9..57ce25f 100755 --- a/src/main/java/org/xerial/snappy/SnappyOutputStream.java +++ b/src/main/java/org/xerial/snappy/SnappyOutputStream.java @@ -86,7 +86,7 @@ public class SnappyOutputStream */ public SnappyOutputStream(OutputStream out, int blockSize) { - this(out, blockSize, CachedBufferAllocator.factory); + this(out, blockSize, CachedBufferAllocator.getBufferAllocatorFactory()); } public SnappyOutputStream(OutputStream out, int blockSize, BufferAllocatorFactory bufferAllocatorFactory) diff --git a/src/main/java/org/xerial/snappy/buffer/CachedBufferAllocator.java b/src/main/java/org/xerial/snappy/buffer/CachedBufferAllocator.java index a0ee40d..3f50f93 100644 --- a/src/main/java/org/xerial/snappy/buffer/CachedBufferAllocator.java +++ b/src/main/java/org/xerial/snappy/buffer/CachedBufferAllocator.java @@ -9,8 +9,7 @@ import java.util.*; public class CachedBufferAllocator implements BufferAllocator { - - public static BufferAllocatorFactory factory = new BufferAllocatorFactory() + private static BufferAllocatorFactory factory = new BufferAllocatorFactory() { @Override public BufferAllocator getBufferAllocator(int bufferSize) @@ -19,10 +18,21 @@ public class CachedBufferAllocator } }; + public static void setBufferAllocatorFactory(BufferAllocatorFactory factory) + { + assert (factory != null); + CachedBufferAllocator.factory = factory; + } + + public static BufferAllocatorFactory getBufferAllocatorFactory() + { + return factory; + } + /** * Use SoftReference so that having this queueTable does not prevent the GC of CachedBufferAllocator instances */ - public static Map> queueTable = new HashMap>(); + private static final Map> queueTable = new HashMap>(); private final int bufferSize; private final Deque bufferQueue; diff --git a/src/test/java/org/xerial/snappy/SnappyOutputStreamTest.java b/src/test/java/org/xerial/snappy/SnappyOutputStreamTest.java index 63e7122..2b8af34 100755 --- a/src/test/java/org/xerial/snappy/SnappyOutputStreamTest.java +++ b/src/test/java/org/xerial/snappy/SnappyOutputStreamTest.java @@ -181,7 +181,7 @@ public class SnappyOutputStreamTest // Regression test for issue #107, a bug where close() was non-idempotent and would release // its buffers to the allocator multiple times, which could cause scenarios where two open // SnappyOutputStreams could share the same buffers, leading to stream corruption issues. - final BufferAllocatorFactory bufferAllocatorFactory = CachedBufferAllocator.factory; + final BufferAllocatorFactory bufferAllocatorFactory = CachedBufferAllocator.getBufferAllocatorFactory(); final int BLOCK_SIZE = 4096; // Create a stream, use it, then close it once: ByteArrayOutputStream ba1 = new ByteArrayOutputStream();