Skip to content

Commit

Permalink
[#5773] AbstractByteBuf.forEachByteDesc(ByteProcessor) starts from wr…
Browse files Browse the repository at this point in the history
…ong index

Motivation:

We introduced a regression in 1abdbe6 which let the iteration start from the wrong index.

Modifications:

Fix start index and add tests.

Result:

Fix regression.
  • Loading branch information
normanmaurer committed Aug 31, 2016
1 parent f97866d commit 7ea5d04
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 1 deletion.
2 changes: 1 addition & 1 deletion buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
Expand Up @@ -1274,7 +1274,7 @@ private int forEachByteAsc0(int start, int end, ByteProcessor processor) throws
public int forEachByteDesc(ByteProcessor processor) {
ensureAccessible();
try {
return forEachByteDesc0(writerIndex, readerIndex, processor);
return forEachByteDesc0(writerIndex - 1, readerIndex, processor);

This comment has been minimized.

Copy link
@johnou

johnou Aug 31, 2016

Contributor

Based off the previous diff shouldn't this read

return forEachByteDesc0(readerIndex, writerIndex -  readerIndex, processor);

This comment has been minimized.

Copy link
@normanmaurer

normanmaurer via email Aug 31, 2016

Author Member
} catch (Exception e) {
PlatformDependent.throwException(e);
return -1;
Expand Down
46 changes: 46 additions & 0 deletions buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
Expand Up @@ -3203,6 +3203,52 @@ public void testReadBytes() {
assertEquals(0, buffer2.refCnt());
}

@Test
public void testForEachByteDesc2() {
byte[] expected = {1, 2, 3, 4};
ByteBuf buf = newBuffer(expected.length);
try {
buf.writeBytes(expected);
final byte[] bytes = new byte[expected.length];
int i = buf.forEachByteDesc(new ByteProcessor() {
private int index = bytes.length - 1;

@Override
public boolean process(byte value) throws Exception {
bytes[index--] = value;
return true;
}
});
assertEquals(-1, i);
assertArrayEquals(expected, bytes);
} finally {
buf.release();
}
}

@Test
public void testForEachByte2() {
byte[] expected = {1, 2, 3, 4};
ByteBuf buf = newBuffer(expected.length);
try {
buf.writeBytes(expected);
final byte[] bytes = new byte[expected.length];
int i = buf.forEachByte(new ByteProcessor() {
private int index;

@Override
public boolean process(byte value) throws Exception {
bytes[index++] = value;
return true;
}
});
assertEquals(-1, i);
assertArrayEquals(expected, bytes);
} finally {
buf.release();
}
}

private static void assertTrueAndRelease(ByteBuf buf, boolean actual) {
try {
assertTrue(actual);
Expand Down
12 changes: 12 additions & 0 deletions buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java
Expand Up @@ -126,6 +126,18 @@ public void testReadBytes() {
// ignore for SlicedByteBuf
}

@Test
@Override
public void testForEachByteDesc2() {
// Ignore for SlicedByteBuf
}

@Test
@Override
public void testForEachByte2() {
// Ignore for SlicedByteBuf
}

@Test(expected = UnsupportedOperationException.class)
@Override
public void testDuplicateCapacityChange() {
Expand Down
Expand Up @@ -123,4 +123,16 @@ public void testRetainedDuplicateCapacityChange() {
public void testLittleEndianWithExpand() {
super.testLittleEndianWithExpand();
}

@Test
@Override
public void testForEachByteDesc2() {
// Ignore
}

@Test
@Override
public void testForEachByte2() {
// Ignore
}
}

0 comments on commit 7ea5d04

Please sign in to comment.