From 0b8f47da0407e3eba78266e65aa389b149da7671 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Tue, 30 Dec 2014 15:52:57 +0900 Subject: [PATCH] Implement internal memory access methods of CompositeByteBuf correctly Motivation: When a CompositeByteBuf is empty (i.e. has no component), its internal memory access operations do not always behave as expected. Modifications: Check if the nunmber of components is zero. If so, return an empty array or an empty NIO buffer, etc. Result: More robustness --- .../io/netty/buffer/CompositeByteBuf.java | 81 +++++++++++++------ 1 file changed, 57 insertions(+), 24 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java index 9785f5baa8..d46f70439b 100644 --- a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java +++ b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java @@ -39,13 +39,13 @@ import java.util.ListIterator; */ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf { + private static final ByteBuffer EMPTY_NIO_BUFFER = Unpooled.EMPTY_BUFFER.nioBuffer(); + private final ResourceLeak leak; private final ByteBufAllocator alloc; private final boolean direct; private final List components = new ArrayList(); private final int maxNumComponents; - private static final ByteBuffer FULL_BYTEBUFFER = (ByteBuffer) ByteBuffer.allocate(1).position(1); - private static final ByteBuffer EMPTY_BYTEBUFFER = ByteBuffer.allocateDirect(0); private boolean freed; @@ -444,50 +444,71 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf { @Override public boolean hasArray() { - if (components.size() == 1) { + switch (components.size()) { + case 0: + return true; + case 1: return components.get(0).buf.hasArray(); + default: + return false; } - return false; } @Override public byte[] array() { - if (components.size() == 1) { + switch (components.size()) { + case 0: + return EmptyArrays.EMPTY_BYTES; + case 1: return components.get(0).buf.array(); + default: + throw new UnsupportedOperationException(); } - throw new UnsupportedOperationException(); } @Override public int arrayOffset() { - if (components.size() == 1) { + switch (components.size()) { + case 0: + return 0; + case 1: return components.get(0).buf.arrayOffset(); + default: + throw new UnsupportedOperationException(); } - throw new UnsupportedOperationException(); } @Override public boolean hasMemoryAddress() { - if (components.size() == 1) { + switch (components.size()) { + case 0: + return Unpooled.EMPTY_BUFFER.hasMemoryAddress(); + case 1: return components.get(0).buf.hasMemoryAddress(); + default: + return false; } - return false; } @Override public long memoryAddress() { - if (components.size() == 1) { + switch (components.size()) { + case 0: + return Unpooled.EMPTY_BUFFER.memoryAddress(); + case 1: return components.get(0).buf.memoryAddress(); + default: + throw new UnsupportedOperationException(); } - throw new UnsupportedOperationException(); } @Override public int capacity() { - if (components.isEmpty()) { + final int numComponents = components.size(); + if (numComponents == 0) { return 0; } - return components.get(components.size() - 1).endOffset; + return components.get(numComponents - 1).endOffset; } @Override @@ -964,7 +985,7 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf { public int setBytes(int index, ScatteringByteChannel in, int length) throws IOException { checkIndex(index, length); if (length == 0) { - return in.read(FULL_BYTEBUFFER); + return in.read(EMPTY_NIO_BUFFER); } int i = toComponentIndex(index); @@ -1093,9 +1114,12 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf { @Override public int nioBufferCount() { - if (components.size() == 1) { + switch (components.size()) { + case 0: + return 1; + case 1: return components.get(0).buf.nioBufferCount(); - } else { + default: int count = 0; int componentsCount = components.size(); for (int i = 0; i < componentsCount; i++) { @@ -1108,26 +1132,35 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf { @Override public ByteBuffer internalNioBuffer(int index, int length) { - if (components.size() == 1) { + switch (components.size()) { + case 0: + return EMPTY_NIO_BUFFER; + case 1: return components.get(0).buf.internalNioBuffer(index, length); + default: + throw new UnsupportedOperationException(); } - throw new UnsupportedOperationException(); } @Override public ByteBuffer nioBuffer(int index, int length) { - if (components.size() == 1) { + checkIndex(index, length); + + switch (components.size()) { + case 0: + return EMPTY_NIO_BUFFER; + case 1: ByteBuf buf = components.get(0).buf; if (buf.nioBufferCount() == 1) { return components.get(0).buf.nioBuffer(index, length); } } + ByteBuffer merged = ByteBuffer.allocate(length).order(order()); ByteBuffer[] buffers = nioBuffers(index, length); - //noinspection ForLoopReplaceableByForEach - for (int i = 0; i < buffers.length; i++) { - merged.put(buffers[i]); + for (ByteBuffer buf: buffers) { + merged.put(buf); } merged.flip(); @@ -1138,7 +1171,7 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf { public ByteBuffer[] nioBuffers(int index, int length) { checkIndex(index, length); if (length == 0) { - return new ByteBuffer[] { EMPTY_BYTEBUFFER }; + return new ByteBuffer[] { EMPTY_NIO_BUFFER }; } List buffers = new ArrayList(components.size());