andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

UnsafeByteBufUtil errors and simplification

Browse Source 
Motiviation:
UnsafeByteBufUtil has some bugs related to using an incorrect index, and also omitting the array paramter when dealing with byte[] objects. There is also some simplification possible with respect to type casting, and minor formatting consistentcy issues.

Modifications:
- Ensure indexing is correct when dealing with native memory
- Fix the native access and endianness for the medium/unsigned medium methods
- Ensure array is used when dealing with heap memory
- Remove unecessary casts when using long
- Fix formating and alignment

Result:
UnsafeByteBufUtil is more correct and won't access direct memory when heap arrays are used.
This commit is contained in:
Scott Mitchell 2017-03-29 16:37:28 -07:00
parent 493a8135f8
commit ef21d5f4ca
2 changed files with 224 additions and 115 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

160
buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java
View File

@ -57,12 +57,9 @@ final class UnsafeByteBufUtil {
static int getUnsignedMedium(long address) {
if (UNALIGNED) {
if (BIG_ENDIAN_NATIVE_ORDER) {
return (PlatformDependent.getByte(address) & 0xff) |
(PlatformDependent.getShort(address + 1) & 0xffff) << 8;
}
return (Short.reverseBytes(PlatformDependent.getShort(address)) & 0xffff) << 8 |
PlatformDependent.getByte(address + 2) & 0xff;
return (PlatformDependent.getByte(address) & 0xff) << 16 |
(BIG_ENDIAN_NATIVE_ORDER ? PlatformDependent.getShort(address + 1)
: Short.reverseBytes(PlatformDependent.getShort(address + 1))) & 0xffff;
}
return (PlatformDependent.getByte(address) & 0xff) << 16 |
(PlatformDependent.getByte(address + 1) & 0xff) << 8 |
@ -71,16 +68,13 @@ final class UnsafeByteBufUtil {
static int getUnsignedMediumLE(long address) {
if (UNALIGNED) {
if (BIG_ENDIAN_NATIVE_ORDER) {
return (Short.reverseBytes(PlatformDependent.getShort(address)) & 0xffff) << 8 |
PlatformDependent.getByte(address + 2) & 0xff;
}
return (PlatformDependent.getByte(address) & 0xff) |
(PlatformDependent.getShort(address + 1) & 0xffff) << 8;
((BIG_ENDIAN_NATIVE_ORDER ? Short.reverseBytes(PlatformDependent.getShort(address + 1))
: PlatformDependent.getShort(address + 1)) & 0xffff) << 8;
}
return PlatformDependent.getByte(address) & 0xff |
(PlatformDependent.getByte(address + 1) & 0xff) << 8 |
(PlatformDependent.getByte(address + 1) & 0xff) << 16 ;
(PlatformDependent.getByte(address + 2) & 0xff) << 16;
}
static int getInt(long address) {
@ -110,14 +104,14 @@ final class UnsafeByteBufUtil {
long v = PlatformDependent.getLong(address);
return BIG_ENDIAN_NATIVE_ORDER ? v : Long.reverseBytes(v);
}
return (long) PlatformDependent.getByte(address) << 56 |
((long) PlatformDependent.getByte(address + 1) & 0xff) << 48 |
((long) PlatformDependent.getByte(address + 2) & 0xff) << 40 |
((long) PlatformDependent.getByte(address + 3) & 0xff) << 32 |
((long) PlatformDependent.getByte(address + 4) & 0xff) << 24 |
((long) PlatformDependent.getByte(address + 5) & 0xff) << 16 |
((long) PlatformDependent.getByte(address + 6) & 0xff) << 8 |
(long) PlatformDependent.getByte(address + 7) & 0xff;
return ((long) PlatformDependent.getByte(address)) << 56 |
(PlatformDependent.getByte(address + 1) & 0xffL) << 48 |
(PlatformDependent.getByte(address + 2) & 0xffL) << 40 |
(PlatformDependent.getByte(address + 3) & 0xffL) << 32 |
(PlatformDependent.getByte(address + 4) & 0xffL) << 24 |
(PlatformDependent.getByte(address + 5) & 0xffL) << 16 |
(PlatformDependent.getByte(address + 6) & 0xffL) << 8 |
(PlatformDependent.getByte(address + 7)) & 0xffL;
}
static long getLongLE(long address) {
@ -125,14 +119,14 @@ final class UnsafeByteBufUtil {
long v = PlatformDependent.getLong(address);
return BIG_ENDIAN_NATIVE_ORDER ? Long.reverseBytes(v) : v;
}
return (long) PlatformDependent.getByte(address) & 0xff |
((long) PlatformDependent.getByte(address + 1) & 0xff) << 8 |
((long) PlatformDependent.getByte(address + 2) & 0xff) << 16 |
((long) PlatformDependent.getByte(address + 3) & 0xff) << 24 |
((long) PlatformDependent.getByte(address + 4) & 0xff) << 32 |
((long) PlatformDependent.getByte(address + 5) & 0xff) << 40 |
((long) PlatformDependent.getByte(address + 6) & 0xff) << 48 |
(long) PlatformDependent.getByte(address + 7) << 56;
return (PlatformDependent.getByte(address)) & 0xffL |
(PlatformDependent.getByte(address + 1) & 0xffL) << 8 |
(PlatformDependent.getByte(address + 2) & 0xffL) << 16 |
(PlatformDependent.getByte(address + 3) & 0xffL) << 24 |
(PlatformDependent.getByte(address + 4) & 0xffL) << 32 |
(PlatformDependent.getByte(address + 5) & 0xffL) << 40 |
(PlatformDependent.getByte(address + 6) & 0xffL) << 48 |
((long) PlatformDependent.getByte(address + 7)) << 56;
}
static void setByte(long address, int value) {
@ -160,32 +154,22 @@ final class UnsafeByteBufUtil {
}
static void setMedium(long address, int value) {
if (UNALIGNED) {
if (BIG_ENDIAN_NATIVE_ORDER) {
PlatformDependent.putByte(address, (byte) value);
PlatformDependent.putShort(address + 1, (short) (value >>> 8));
} else {
PlatformDependent.putShort(address, Short.reverseBytes((short) (value >>> 8)));
PlatformDependent.putByte(address + 2, (byte) value);
}
} else {
PlatformDependent.putByte(address, (byte) (value >>> 16));
if (UNALIGNED) {
PlatformDependent.putShort(address + 1, BIG_ENDIAN_NATIVE_ORDER ? (short) value
: Short.reverseBytes((short) value));
} else {
PlatformDependent.putByte(address + 1, (byte) (value >>> 8));
PlatformDependent.putByte(address + 2, (byte) value);
}
}
static void setMediumLE(long address, int value) {
PlatformDependent.putByte(address, (byte) value);
if (UNALIGNED) {
if (BIG_ENDIAN_NATIVE_ORDER) {
PlatformDependent.putShort(address, Short.reverseBytes((short) (value >>> 8)));
PlatformDependent.putByte(address + 2, (byte) value);
PlatformDependent.putShort(address + 1, BIG_ENDIAN_NATIVE_ORDER ? Short.reverseBytes((short) (value >>> 8))
: (short) (value >>> 8));
} else {
PlatformDependent.putByte(address, (byte) value);
PlatformDependent.putShort(address + 1, (short) (value >>> 8));
}
} else {
PlatformDependent.putByte(address, (byte) value);
PlatformDependent.putByte(address + 1, (byte) (value >>> 8));
PlatformDependent.putByte(address + 2, (byte) (value >>> 16));
}
@ -261,17 +245,16 @@ final class UnsafeByteBufUtil {
short v = PlatformDependent.getShort(array, index);
return BIG_ENDIAN_NATIVE_ORDER ? Short.reverseBytes(v) : v;
}
return (short) (PlatformDependent.getByte(index) & 0xff | PlatformDependent.getByte(index + 1) << 8);
return (short) (PlatformDependent.getByte(array, index) & 0xff |
PlatformDependent.getByte(array, index + 1) << 8);
}
static int getUnsignedMedium(byte[] array, int index) {
if (UNALIGNED) {
if (BIG_ENDIAN_NATIVE_ORDER) {
return (PlatformDependent.getByte(array, index) & 0xff) |
(PlatformDependent.getShort(array, index + 1) & 0xffff) << 8;
}
return (Short.reverseBytes(PlatformDependent.getShort(array, index)) & 0xffff) << 8 |
PlatformDependent.getByte(array, index + 2) & 0xff;
return (PlatformDependent.getByte(array, index) & 0xff) << 16 |
(BIG_ENDIAN_NATIVE_ORDER ? PlatformDependent.getShort(array, index + 1)
: Short.reverseBytes(PlatformDependent.getShort(array, index + 1)))
& 0xffff;
}
return (PlatformDependent.getByte(array, index) & 0xff) << 16 |
(PlatformDependent.getByte(array, index + 1) & 0xff) << 8 |
@ -280,12 +263,9 @@ final class UnsafeByteBufUtil {
static int getUnsignedMediumLE(byte[] array, int index) {
if (UNALIGNED) {
if (BIG_ENDIAN_NATIVE_ORDER) {
return (Short.reverseBytes(PlatformDependent.getShort(array, index)) & 0xffff) << 8 |
PlatformDependent.getByte(array, index + 2) & 0xff;
}
return (PlatformDependent.getByte(array, index) & 0xff) |
(PlatformDependent.getShort(array, index + 1) & 0xffff) << 8;
((BIG_ENDIAN_NATIVE_ORDER ? Short.reverseBytes(PlatformDependent.getShort(array, index + 1))
: PlatformDependent.getShort(array, index + 1)) & 0xffff) << 8;
}
return PlatformDependent.getByte(array, index) & 0xff |
(PlatformDependent.getByte(array, index + 1) & 0xff) << 8 |
@ -319,14 +299,14 @@ final class UnsafeByteBufUtil {
long v = PlatformDependent.getLong(array, index);
return BIG_ENDIAN_NATIVE_ORDER ? v : Long.reverseBytes(v);
}
return (long) PlatformDependent.getByte(array, index) << 56 |
((long) PlatformDependent.getByte(array, index + 1) & 0xff) << 48 |
((long) PlatformDependent.getByte(array, index + 2) & 0xff) << 40 |
((long) PlatformDependent.getByte(array, index + 3) & 0xff) << 32 |
((long) PlatformDependent.getByte(array, index + 4) & 0xff) << 24 |
((long) PlatformDependent.getByte(array, index + 5) & 0xff) << 16 |
((long) PlatformDependent.getByte(array, index + 6) & 0xff) << 8 |
(long) PlatformDependent.getByte(array, index + 7) & 0xff;
return ((long) PlatformDependent.getByte(array, index)) << 56 |
(PlatformDependent.getByte(array, index + 1) & 0xffL) << 48 |
(PlatformDependent.getByte(array, index + 2) & 0xffL) << 40 |
(PlatformDependent.getByte(array, index + 3) & 0xffL) << 32 |
(PlatformDependent.getByte(array, index + 4) & 0xffL) << 24 |
(PlatformDependent.getByte(array, index + 5) & 0xffL) << 16 |
(PlatformDependent.getByte(array, index + 6) & 0xffL) << 8 |
(PlatformDependent.getByte(array, index + 7)) & 0xffL;
}
static long getLongLE(byte[] array, int index) {
@ -334,14 +314,14 @@ final class UnsafeByteBufUtil {
long v = PlatformDependent.getLong(array, index);
return BIG_ENDIAN_NATIVE_ORDER ? Long.reverseBytes(v) : v;
}
return (long) PlatformDependent.getByte(array, index) & 0xff |
((long) PlatformDependent.getByte(array, index + 1) & 0xff) << 8 |
((long) PlatformDependent.getByte(array, index + 2) & 0xff) << 16 |
((long) PlatformDependent.getByte(array, index + 3) & 0xff) << 24 |
((long) PlatformDependent.getByte(array, index + 4) & 0xff) << 32 |
((long) PlatformDependent.getByte(array, index + 5) & 0xff) << 40 |
((long) PlatformDependent.getByte(array, index + 6) & 0xff) << 48 |
(long) PlatformDependent.getByte(array, index + 7) << 56;
return PlatformDependent.getByte(array, index) & 0xffL |
(PlatformDependent.getByte(array, index + 1) & 0xffL) << 8 |
(PlatformDependent.getByte(array, index + 2) & 0xffL) << 16 |
(PlatformDependent.getByte(array, index + 3) & 0xffL) << 24 |
(PlatformDependent.getByte(array, index + 4) & 0xffL) << 32 |
(PlatformDependent.getByte(array, index + 5) & 0xffL) << 40 |
(PlatformDependent.getByte(array, index + 6) & 0xffL) << 48 |
((long) PlatformDependent.getByte(array, index + 7)) << 56;
}
static void setByte(byte[] array, int index, int value) {
@ -350,8 +330,8 @@ final class UnsafeByteBufUtil {
static void setShort(byte[] array, int index, int value) {
if (UNALIGNED) {
PlatformDependent.putShort(
array, index, BIG_ENDIAN_NATIVE_ORDER ? (short) value : Short.reverseBytes((short) value));
PlatformDependent.putShort(array, index,
BIG_ENDIAN_NATIVE_ORDER ? (short) value : Short.reverseBytes((short) value));
} else {
PlatformDependent.putByte(array, index, (byte) (value >>> 8));
PlatformDependent.putByte(array, index + 1, (byte) value);
@ -360,8 +340,8 @@ final class UnsafeByteBufUtil {
static void setShortLE(byte[] array, int index, int value) {
if (UNALIGNED) {
PlatformDependent.putShort(
array, index, BIG_ENDIAN_NATIVE_ORDER ? Short.reverseBytes((short) value) : (short) value);
PlatformDependent.putShort(array, index,
BIG_ENDIAN_NATIVE_ORDER ? Short.reverseBytes((short) value) : (short) value);
} else {
PlatformDependent.putByte(array, index, (byte) value);
PlatformDependent.putByte(array, index + 1, (byte) (value >>> 8));
@ -369,32 +349,24 @@ final class UnsafeByteBufUtil {
}
static void setMedium(byte[] array, int index, int value) {
if (UNALIGNED) {
if (BIG_ENDIAN_NATIVE_ORDER) {
PlatformDependent.putByte(array, index, (byte) value);
PlatformDependent.putShort(array, index + 1, (short) (value >>> 8));
} else {
PlatformDependent.putShort(array, index, Short.reverseBytes((short) (value >>> 8)));
PlatformDependent.putByte(array, index + 2, (byte) value);
}
} else {
PlatformDependent.putByte(array, index, (byte) (value >>> 16));
if (UNALIGNED) {
PlatformDependent.putShort(array, index + 1,
BIG_ENDIAN_NATIVE_ORDER ? (short) value
: Short.reverseBytes((short) value));
} else {
PlatformDependent.putByte(array, index + 1, (byte) (value >>> 8));
PlatformDependent.putByte(array, index + 2, (byte) value);
}
}
static void setMediumLE(byte[] array, int index, int value) {
PlatformDependent.putByte(array, index, (byte) value);
if (UNALIGNED) {
if (BIG_ENDIAN_NATIVE_ORDER) {
PlatformDependent.putShort(array, index, Short.reverseBytes((short) (value >>> 8)));
PlatformDependent.putByte(array, index + 2, (byte) value);
PlatformDependent.putShort(array, index + 1,
BIG_ENDIAN_NATIVE_ORDER ? Short.reverseBytes((short) (value >>> 8))
: (short) (value >>> 8));
} else {
PlatformDependent.putByte(array, index, (byte) value);
PlatformDependent.putShort(array, index + 1, (short) (value >>> 8));
}
} else {
PlatformDependent.putByte(array, index, (byte) value);
PlatformDependent.putByte(array, index + 1, (byte) (value >>> 8));
PlatformDependent.putByte(array, index + 2, (byte) (value >>> 16));
}

137
buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
View File

@ -71,6 +71,7 @@ public abstract class AbstractByteBufTest {
private static final int CAPACITY = 4096; // Must be even
private static final int BLOCK_SIZE = 128;
private static final int JAVA_BYTEBUFFER_CONSISTENCY_ITERATIONS = 100;
private long seed;
private Random random;
@ -459,6 +460,40 @@ public abstract class AbstractByteBufTest {
}
}
@Test
public void testShortConsistentWithByteBuffer() {
testShortConsistentWithByteBuffer(true, true);
testShortConsistentWithByteBuffer(true, false);
testShortConsistentWithByteBuffer(false, true);
testShortConsistentWithByteBuffer(false, false);
}
private void testShortConsistentWithByteBuffer(boolean direct, boolean testBigEndian) {
for (int i = 0; i < JAVA_BYTEBUFFER_CONSISTENCY_ITERATIONS; ++i) {
ByteBuffer javaBuffer = direct ? ByteBuffer.allocateDirect(buffer.capacity())
: ByteBuffer.allocate(buffer.capacity());
if (!testBigEndian) {
javaBuffer = javaBuffer.order(ByteOrder.LITTLE_ENDIAN);
}
short expected = (short) (random.nextInt() & 0xFFFF);
javaBuffer.putShort(expected);
final int bufferIndex = buffer.capacity() - 2;
if (testBigEndian) {
buffer.setShort(bufferIndex, expected);
} else {
buffer.setShortLE(bufferIndex, expected);
}
javaBuffer.flip();
short javaActual = javaBuffer.getShort();
assertEquals(expected, javaActual);
assertEquals(javaActual, testBigEndian ? buffer.getShort(bufferIndex)
: buffer.getShortLE(bufferIndex));
}
}
@Test
public void testRandomUnsignedShortAccess() {
testRandomUnsignedShortAccess(true);
@ -552,6 +587,40 @@ public abstract class AbstractByteBufTest {
}
}
@Test
public void testMediumConsistentWithByteBuffer() {
testMediumConsistentWithByteBuffer(true, true);
testMediumConsistentWithByteBuffer(true, false);
testMediumConsistentWithByteBuffer(false, true);
testMediumConsistentWithByteBuffer(false, false);
}
private void testMediumConsistentWithByteBuffer(boolean direct, boolean testBigEndian) {
for (int i = 0; i < JAVA_BYTEBUFFER_CONSISTENCY_ITERATIONS; ++i) {
ByteBuffer javaBuffer = direct ? ByteBuffer.allocateDirect(buffer.capacity())
: ByteBuffer.allocate(buffer.capacity());
if (!testBigEndian) {
javaBuffer = javaBuffer.order(ByteOrder.LITTLE_ENDIAN);
}
int expected = random.nextInt() & 0x00FFFFFF;
javaBuffer.putInt(expected);
final int bufferIndex = buffer.capacity() - 3;
if (testBigEndian) {
buffer.setMedium(bufferIndex, expected);
} else {
buffer.setMediumLE(bufferIndex, expected);
}
javaBuffer.flip();
int javaActual = javaBuffer.getInt();
assertEquals(expected, javaActual);
assertEquals(javaActual, testBigEndian ? buffer.getUnsignedMedium(bufferIndex)
: buffer.getUnsignedMediumLE(bufferIndex));
}
}
@Test
public void testRandomIntAccess() {
testRandomIntAccess(true);
@ -583,6 +652,40 @@ public abstract class AbstractByteBufTest {
}
}
@Test
public void testIntConsistentWithByteBuffer() {
testIntConsistentWithByteBuffer(true, true);
testIntConsistentWithByteBuffer(true, false);
testIntConsistentWithByteBuffer(false, true);
testIntConsistentWithByteBuffer(false, false);
}
private void testIntConsistentWithByteBuffer(boolean direct, boolean testBigEndian) {
for (int i = 0; i < JAVA_BYTEBUFFER_CONSISTENCY_ITERATIONS; ++i) {
ByteBuffer javaBuffer = direct ? ByteBuffer.allocateDirect(buffer.capacity())
: ByteBuffer.allocate(buffer.capacity());
if (!testBigEndian) {
javaBuffer = javaBuffer.order(ByteOrder.LITTLE_ENDIAN);
}
int expected = random.nextInt();
javaBuffer.putInt(expected);
final int bufferIndex = buffer.capacity() - 4;
if (testBigEndian) {
buffer.setInt(bufferIndex, expected);
} else {
buffer.setIntLE(bufferIndex, expected);
}
javaBuffer.flip();
int javaActual = javaBuffer.getInt();
assertEquals(expected, javaActual);
assertEquals(javaActual, testBigEndian ? buffer.getInt(bufferIndex)
: buffer.getIntLE(bufferIndex));
}
}
@Test
public void testRandomUnsignedIntAccess() {
testRandomUnsignedIntAccess(true);
@ -645,6 +748,40 @@ public abstract class AbstractByteBufTest {
}
}
@Test
public void testLongConsistentWithByteBuffer() {
testLongConsistentWithByteBuffer(true, true);
testLongConsistentWithByteBuffer(true, false);
testLongConsistentWithByteBuffer(false, true);
testLongConsistentWithByteBuffer(false, false);
}
private void testLongConsistentWithByteBuffer(boolean direct, boolean testBigEndian) {
for (int i = 0; i < JAVA_BYTEBUFFER_CONSISTENCY_ITERATIONS; ++i) {
ByteBuffer javaBuffer = direct ? ByteBuffer.allocateDirect(buffer.capacity())
: ByteBuffer.allocate(buffer.capacity());
if (!testBigEndian) {
javaBuffer = javaBuffer.order(ByteOrder.LITTLE_ENDIAN);
}
long expected = random.nextLong();
javaBuffer.putLong(expected);
final int bufferIndex = buffer.capacity() - 8;
if (testBigEndian) {
buffer.setLong(bufferIndex, expected);
} else {
buffer.setLongLE(bufferIndex, expected);
}
javaBuffer.flip();
long javaActual = javaBuffer.getLong();
assertEquals(expected, javaActual);
assertEquals(javaActual, testBigEndian ? buffer.getLong(bufferIndex)
: buffer.getLongLE(bufferIndex));
}
}
@Test
public void testRandomFloatAccess() {
for (int i = 0; i < buffer.capacity() - 7; i += 8) {