ctubbsii commented on code in PR #4745:
URL: https://github.com/apache/accumulo/pull/4745#discussion_r1692516093


##########
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:
   @keith-turner identified [on another 
issue](https://github.com/apache/accumulo/pull/4524#discussion_r1692342340) 
that Java's built-in 
[Objects](https://docs.oracle.com/en%2Fjava%2Fjavase%2F11%2Fdocs%2Fapi%2F%2F/java.base/java/util/Objects.html)
 has some methods to check bounds as well.
   
   The exceptions thrown for each of the 3 (what we have, Objects, 
Preconditions) are all slightly different, but are all RuntimeExceptions, so 
fundamentally they all should check things the same way... it would only matter 
if somebody had a specific catch block checking for which RTE it was.
   
   Of the options, I'm inclined to choose [the built-in Objects version][1].
   
   [1]: 
https://docs.oracle.com/en%2Fjava%2Fjavase%2F11%2Fdocs%2Fapi%2F%2F/java.base/java/util/Objects.html#checkFromIndexSize(int,int,int)
 



-- 
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]

Reply via email to