dweiss commented on a change in pull request #348:
URL: https://github.com/apache/lucene/pull/348#discussion_r720782981
##########
File path:
lucene/core/src/java/org/apache/lucene/store/ByteBuffersDataInput.java
##########
@@ -103,6 +103,24 @@ public byte readByte() throws EOFException {
}
}
+ @Override
+ public short readShort() throws IOException {
+ // TODO: use ByteBuffer.getShort
Review comment:
+ add an assertion somewhere ensuring proper byte order on all blocks?
##########
File path:
lucene/core/src/java/org/apache/lucene/codecs/MultiLevelSkipListReader.java
##########
@@ -319,6 +320,27 @@ public void readBytes(byte[] b, int offset, int len) {
pos += len;
}
+ @Override
+ public short readShort() throws IOException {
+ short value = (short) BitUtil.VH_LE_SHORT.get(pos);
+ pos += Short.BYTES;
+ return value;
+ }
+
+ @Override
+ public int readInt() throws IOException {
+ int value = (int) BitUtil.VH_LE_INT.get(pos);
+ pos += Integer.BYTES;
+ return value;
+ }
+
+ @Override
+ public long readLong() throws IOException {
Review comment:
All multibyte-reading implementations with position increment after the
read suffer from the same subtle issue: imagine you're at file length-3 bytes
offset and trying to read an int: readInt() would fail but a subsequent
readByte() or readShort() would succeed. I'm not sure if we need to make it a
contract that the bytes are always consumed by readXYZ but if we don't then
it'd be good to document this somewhere (even as an "unspecified behavior on
insufficient bytes").
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
