Repository: netty/netty
Issue: CompositeChannelBuffer: Add expert method to return the ChannelBuffer for a specific index

It would be nice to add an expert method to CompositeChannelBuffer that allows to get the ChannelBuffer for a specific index. Pretty simple to add, would help a lot in my case when trying to optimize buffer copies when possible.

If it makes sense, I can easily send a pull request.


Issue comments:
I understand the probable need.
Here are some remarks on my side:

1- a clear documentation must indicate that the returned ByteBuf is limited to the portion of bytes where the index given is physically located

2- why do you make 

```
return components[componentId].duplicate();
```

instead of

```
return components[componentId];
```

SORRY: bad interpretation of duplicate, but still point 2 and 3 together are valid comments...

> erroneous: This should even limitated the in-memory copy process... But if you do need a copy, then maybe you should explicitely duplicate it from the output, not from the method ? 

3- Moreover, I think duplicate() is changing the index of the duplicated ByteBuf (0, buf.capacity), instead of

```
 ByteBuf buf = components[componentId];
 return buf.slice(buf.readerIndex(), buf.readableBytes());
```

What do you think ?

Agreed on the documentation, it should be stated that this is an expert method. As such, btw, I don't see a reason to duplicate the buffer, if the "expert" in this case wishes to, he can do it.

As for the implementation, it an overhead something like this will work:

```
    return components(componentId(index));
```

Thanks, guys! I've pushed up a new commit with your suggestions.

I can't make the change for proposal from where I am now, but I do agree with @kimchy comment:

```
return components(componentId(index));
```

And for the comment, could be something like:

```
/**
* Return the underlying ByteBuf region that contains the index element. Expert method only
*
* Modifying the content of the returned buffer or this buffer affects
* each other's content. They do shared the same indexes and marks.
* It is therefore recommended to use a slice() of the returned buffer except
* if you know what you are doing.
*
* @param index the index that is included in the returned ByteBuf
* @return the ByteBuf containing the index
* 
* @throws IndexOutOfBoundsException
* if the specified {@code index} is less than {@code this.readerIndex()} or
* is greater than {@code this.readerIndex()+this.readableBytes()}
*/
```

Also I come back with the initial test (that throws the exception).

Is it ok to test from 0 to capacity or should it be between global readerIndex and global readableBytes ?
I believe that you do want a piece of buffer that is valid (and no more sliced out of the composite buffer) ?

@CruzBishop so according to me it is fine
@trustin or @normanmaurer , are you ok with this proposal ?

Seems to be fine - You know new code, it works until somebody abuses it :P

Will review later today

Sent from my iPhone. Excuse any typos....

Am 29.06.2012 um 10:15 schrieb Frédéric Brégierreply@reply.github.com:

> @CruzBishop so according to me it is fine
> @trustin or @normanmaurer , are you ok with this proposal ?
> 
> ---
> 
> Reply to this email directly or view it on GitHub:
> https://github.com/netty/netty/issues/414#issuecomment-6654512

Thanks, Norman!


Pull request review comments:

Patch that resolved the issue:
diff --git a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
index 9329de90231..ca86eb3cd06 100644
--- a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
+++ b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
@@ -575,6 +575,34 @@ private void copyTo(int index, int length, int componentId, ByteBuf dst) {
         dst.writerIndex(dst.capacity());
     }
 
+    /**
+     * Returns the {@link ByteBuf} portion of this {@link CompositeByteBuf} that
+     * contains the specified {@code index}. This is an expert method!
+     *
+     * <p>
+     * Please note that since a {@link CompositeByteBuf} is made up of
+     * multiple {@link ByteBuf}s, this does <em>not</em> return the full buffer.
+     * Instead, it only returns a portion of the composite buffer where the
+     * index is located
+     * </p>
+     *
+     * @param index The {@code index} to search for and include in the returned {@link ByteBuf}
+     * @return The {@link ByteBuf} that contains the specified {@code index}
+     * @throws IndexOutOfBoundsException when the specified {@code index} is
+     * less than zero, or larger than {@code capacity()}
+     */
+    public ByteBuf getBufferFor(int index) throws IndexOutOfBoundsException {
+        if (index < 0 || index >= capacity()) {
+            throw new IndexOutOfBoundsException("Invalid index: " + index
+                    + " - Bytes needed: " + index + ", maximum is "
+                    + capacity());
+        }
+
+        //Return the component byte buffer
+        return components[componentId(index)];
+
+    }
+
     @Override
     public ByteBuf slice(int index, int length) {
         if (index == 0) {


Test patch:
diff --git a/buffer/src/test/java/io/netty/buffer/AbstractCompositeChannelBufferTest.java b/buffer/src/test/java/io/netty/buffer/AbstractCompositeChannelBufferTest.java
index 2d68c5cd1d3..86fa2a50098 100644
--- a/buffer/src/test/java/io/netty/buffer/AbstractCompositeChannelBufferTest.java
+++ b/buffer/src/test/java/io/netty/buffer/AbstractCompositeChannelBufferTest.java
@@ -16,6 +16,7 @@
 package io.netty.buffer;
 
 import static io.netty.buffer.Unpooled.*;
+import java.io.IOException;
 import static org.junit.Assert.*;
 
 import java.nio.ByteBuffer;
@@ -89,6 +90,29 @@ protected ByteBuf[] components() {
     protected boolean discardReadBytesDoesNotMoveWritableBytes() {
         return false;
     }
+    
+    /**
+     * Tests the "getBufferFor" method
+     */
+    @Test
+    public void testGetBufferFor() throws IOException {
+        CompositeByteBuf buf = (CompositeByteBuf) Unpooled.wrappedBuffer(new byte[] { 1, 2, 3, 4, 5 }, new byte[] {4, 5, 6, 7, 8, 9, 26});
+    
+        //Ensure that a random place will be fine
+        assertEquals(buf.getBufferFor(2).capacity(), 5);
+        
+        //Loop through each byte
+        
+        byte index = 0;
+        
+        while (index < buf.capacity()) {
+            ByteBuf _buf = buf.getBufferFor(index++);
+            assertNotNull(_buf);
+            assertTrue(_buf.capacity() > 0);
+            assertNotNull(_buf.getByte(0));
+            assertNotNull(_buf.getByte(_buf.readableBytes() - 1));
+        }
+    }
 
     @Test
     public void testDiscardReadBytes3() {