DomGarguilo commented on code in PR #3219:
URL: https://github.com/apache/accumulo/pull/3219#discussion_r1139163864
##########
core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/SeekableByteArrayInputStream.java:
##########
@@ -124,26 +145,24 @@ public void close() throws IOException {}
public SeekableByteArrayInputStream(byte[] buf) {
requireNonNull(buf, "bug argument was null");
this.buffer = buf;
- this.cur = 0;
this.max = buf.length;
}
public SeekableByteArrayInputStream(byte[] buf, int maxOffset) {
requireNonNull(buf, "bug argument was null");
this.buffer = buf;
- this.cur = 0;
this.max = maxOffset;
}
public void seek(int position) {
if (position < 0 || position >= max) {
throw new IllegalArgumentException("position = " + position + "
maxOffset = " + max);
}
- this.cur = position;
+ this.cur.set(position);
}
public int getPosition() {
- return this.cur;
+ return this.cur.get();
}
byte[] getBuffer() {
Review Comment:
It looks like this is only used in one place.
`SeekableByteArrayInputStream.getBuffer()` is used in
`CachableBlockFile.CachedBlockRead.getBuffer()` which is only called in
`MultiLevelIndex.IndexBlock.readFields()`. There is a comment that says `this
block is cached, so avoid copy` in `readFields()` so making a copy to avoid
altering the original buffer might not be an option here. As far as I can tell
though, the byte[] returned by `getBuffer()` is never altered, only read, so it
might not be an issue in the current code. Would be nice to make it immutable
but there is no immutable byte array in java.
I'm not too sure how your concern should be addressed @ctubbsii. Do you have
any ideas? If not do you think this PR is ready to merge?
--
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]
