ctubbsii commented on code in PR #4745:
URL: https://github.com/apache/accumulo/pull/4745#discussion_r1691955330
##########
core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java:
##########
@@ -1055,7 +1054,7 @@ private void _seek(Range range) throws IOException {
hasTop = true;
}
- MutableByteSequence valbs = new MutableByteSequence(new byte[64], 0,
0);
+ ArrayByteSequence valbs = new ArrayByteSequence(new byte[64], 0, 0);
Review Comment:
These actually should be final too, to ensure we're reusing the object, like
we intended.
##########
core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java:
##########
@@ -1072,7 +1071,7 @@ private void _seek(Range range) throws IOException {
val = new Value();
val.readFields(currBlock);
- valbs = new MutableByteSequence(val.get(), 0, val.getSize());
+ valbs = new ArrayByteSequence(val.get(), 0, val.getSize());
Review Comment:
If this variable is made final, then this needs to call reset to change the
backing array.
##########
core/src/main/java/org/apache/accumulo/core/data/ArrayByteSequence.java:
##########
@@ -160,6 +160,10 @@ public int offset() {
* @since 3.1.0
*/
public void reset(byte[] data, int offset, int length) {
+ if (offset < 0 || offset > data.length || length < 0 || (offset + length)
> data.length) {
+ throw new IllegalArgumentException(" Bad offset and/or length
data.length = " + data.length
+ + " offset = " + offset + " length = " + length);
+ }
Review Comment:
Could use a static private method so this code is reused between the
constructor and here. Also could use Guava's [Preconditions to check
indexes](https://guava.dev/releases/19.0/api/docs/com/google/common/base/Preconditions.html),
which is already a static method ready to use. I think they have something
that's easy to do that with, and we've used Preconditions elsewhere in our
code. We just have to make sure it's a stable API and not beta or something.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
