Copilot commented on code in PR #2324:
URL: https://github.com/apache/pekko/pull/2324#discussion_r3035535738
##########
actor/src/main/scala/org/apache/pekko/util/SWARUtil.scala:
##########
@@ -91,4 +92,67 @@ private[util] object SWARUtil {
(array(index + 7).toLong & 0xFF)
}
}
+
+ // Derived from code in Netty
+ //
https://github.com/netty/netty/blob/d28a0fc6598b50fbe8f296831777cf4b653a475f/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java#L366-L408
+ final def arrayBytesMatch(arrayBytes: Array[Byte],
+ fromIndex: Int,
+ checkBytes: Array[Byte],
+ bytesFromIndex: Int,
+ checkLength: Int): Boolean = {
+ var aIndex = fromIndex
+ var bIndex = bytesFromIndex
+ val longCount = checkLength >>> 3
+ val byteCount = checkLength & 7
+ var i = 0
+ while (i < longCount) {
+ if (getLong(arrayBytes, aIndex) != getLong(checkBytes, bIndex)) return
false
+ aIndex += 8
+ bIndex += 8
+ i += 1
+ }
+ i = 0
+ while (i < byteCount) {
+ if (arrayBytes(aIndex) != checkBytes(bIndex)) return false
+ aIndex += 1
+ bIndex += 1
+ i += 1
+ }
+ true
+ }
+
+ // Derived from code in Netty
+ //
https://github.com/netty/netty/blob/a5343227b10456ec889a3fdc5fa4246f036a216d/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java#L327-L356
+ final def maxSuf(arrayBytes: Array[Byte], m: Int, start: Int, isSuffix:
Boolean): Long = {
+ var p = 1
+ var ms = -1
+ var j = start
+ var k = 1
+ var a = 0
+ var b = 0
+ while (j + k < m) {
+ a = arrayBytes(j + k)
+ b = arrayBytes(ms + k)
+ val suffix = if (isSuffix) a < b
+ else a > b
+ if (suffix) {
+ j += k
+ k = 1
+ p = j - ms
+ } else if (a == b) {
+ if (k != p) {
+ k += 1;
+ } else {
+ j += p;
+ k = 1;
Review Comment:
`SWARUtil.maxSuf` introduces Java-style semicolons (`k += 1;`, `j += p;`, `k
= 1;`) which is inconsistent with the surrounding Scala style and makes diffs
noisier. Drop the semicolons for consistency.
```suggestion
k += 1
} else {
j += p
k = 1
```
##########
actor/src/main/scala/org/apache/pekko/util/ByteString.scala:
##########
@@ -575,6 +668,86 @@ object ByteString {
else -1
}
+ // Derived from code in Netty
+ //
https://github.com/netty/netty/blob/d28a0fc6598b50fbe8f296831777cf4b653a475f/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java#L242-L325
+ override def indexOfSlice(slice: Array[Byte], from: Int): Int = {
+ val n = length - from
+ val m = slice.length
+ if (m == 0) return 0
+ // When the needle has only one byte that can be read,
+ // the indexOf() can be used
+ if (m == 1) return indexOf(slice.head, from)
+ var i = 0
+ var j = 0
+ val aStartIndex = 0
+ val bStartIndex = from + startIndex
+ val suffixes = SWARUtil.maxSuf(slice, m, aStartIndex, true)
Review Comment:
`ByteString1.indexOfSlice` uses `from` directly (via `n = length - from` and
`bStartIndex = from + startIndex`) without clamping negative values. This is a
behavior change vs the base implementation (`rec(math.max(0, from))`) and can
produce negative indices into the backing array when `from < 0`, throwing
`ArrayIndexOutOfBoundsException`. Clamp `from` to 0 at the start of the method
and base all subsequent calculations on the clamped value.
##########
actor/src/main/scala/org/apache/pekko/util/ByteString.scala:
##########
@@ -912,6 +1085,22 @@ object ByteString {
}
}
+ private[util] def bytesMatch(fromIndex: Int,
+ checkBytes: Array[Byte],
+ checkBytesFromIndex: Int,
+ checkLength: Int): Boolean = {
+ if (checkLength > 1 && bytestrings.nonEmpty && bytestrings.head.length
>= fromIndex + checkLength - 1) {
Review Comment:
`ByteStrings.bytesMatch` has an off-by-one bounds check when deciding
whether it can delegate to `bytestrings.head.bytesMatch(...)`. The condition
`bytestrings.head.length >= fromIndex + checkLength - 1` will be true even when
the head segment is 1 byte too short, which can lead to
`ArrayIndexOutOfBoundsException` inside the optimized matcher. Update the
condition to require that the head contains *all* requested bytes (i.e.,
`head.length >= fromIndex + checkLength`).
```suggestion
if (checkLength > 1 && bytestrings.nonEmpty && bytestrings.head.length
>= fromIndex + checkLength) {
```
##########
actor/src/main/scala/org/apache/pekko/util/ByteString.scala:
##########
@@ -314,6 +329,84 @@ object ByteString {
else -1
}
+ // Derived from code in Netty
+ //
https://github.com/netty/netty/blob/d28a0fc6598b50fbe8f296831777cf4b653a475f/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java#L242-L325
+ override def indexOfSlice(slice: Array[Byte], from: Int): Int = {
+ val n = length - from
+ val m = slice.length
+ if (m == 0) return 0
+ // When the needle has only one byte that can be read,
+ // the indexOf() can be used
+ if (m == 1) return indexOf(slice.head, from)
+ var i = 0
+ var j = 0
+ val aStartIndex = 0
+ val bStartIndex = from
Review Comment:
`ByteString1C.indexOfSlice` uses `from` directly to compute `n` and to index
into `bytes` (via `bStartIndex = from`). Unlike the base
`ByteString.indexOfSlice`, this override does not clamp negative `from` values
to 0, so `from < 0` can cause negative array indices and
`ArrayIndexOutOfBoundsException`. Consider introducing `val fromIndex =
math.max(0, from)` and using that consistently for `n`/`bStartIndex`/return
values.
```suggestion
val fromIndex = math.max(0, from)
val n = length - fromIndex
val m = slice.length
if (m == 0) return 0
// When the needle has only one byte that can be read,
// the indexOf() can be used
if (m == 1) return indexOf(slice.head, fromIndex)
var i = 0
var j = 0
val aStartIndex = 0
val bStartIndex = fromIndex
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
