Issue/277 #285
Issue/277 #285
Conversation
|
Kudos, SonarCloud Quality Gate passed! |
| @@ -273,12 +277,14 @@ public long capacity() { | |||
| @Override | |||
| public long addressForRead(long offset) | |||
There was a problem hiding this comment.
Should we add @nonnegative annotation to these for consistency?
There was a problem hiding this comment.
Agree. I believe that offsets should always be from the start of the Bytes and thus always be >= 0
There was a problem hiding this comment.
I think we should. This makes the contract clearer. I have raised #290
| @@ -273,12 +277,14 @@ public long capacity() { | |||
| @Override | |||
| public long addressForRead(long offset) | |||
| throws UnsupportedOperationException, IllegalStateException, BufferUnderflowException { | |||
| requireNonNegative(offset); | |||
| return base.addressForRead(offset); | |||
| } | |||
|
|
|||
| @Override | |||
| public long addressForWrite(long offset) | |||
| @@ -881,6 +896,7 @@ public void readWithLength(Bytes bytes) | |||
| @NotNull | |||
| public Bytes<Void> writePosition(long position) | |||
| @@ -1106,11 +1122,11 @@ private void doIndent() { | |||
|
|
|||
| @Override | |||
| @NotNull | |||
| public Bytes<Void> write(@NotNull byte[] bytes, int offset, int length) | |||
| public Bytes<Void> write(@NotNull byte[] byteArray, int offset, int length) | |||
| @@ -1655,6 +1674,9 @@ public Bytes write(@NotNull InputStream inputStream) | |||
| @Override | |||
| public Bytes<Void> write(@NotNull BytesStore bytes, long offset, long length) | |||
| @@ -1709,6 +1731,8 @@ public Bytes write(@NotNull InputStream inputStream) | |||
| @Override | |||
| public void writePositionRemaining(long position, long length) | |||
|
Seemed to be some inconsistencies with our use of the annotations, but other than that LGTM! |
| final byte[] byteArray, | ||
| @NonNegative final int offset, | ||
| @NonNegative final int length) throws IllegalStateException { | ||
| requireNonNegative(offsetInRDO); |
There was a problem hiding this comment.
My dream is that we (and customers) can validate @NonNegative etc at compile time using static analysis, and we can turn off checks at runtime
There was a problem hiding this comment.
This is absolutely doable. We could clone and modify the current non-null plugin.
This PR introduces additional tests and checkings around negative values. This prevents a number of bugs and JVM crashes in the tests.