divijvaidya commented on code in PR #12948:
URL: https://github.com/apache/kafka/pull/12948#discussion_r1242328144
##########
clients/src/main/java/org/apache/kafka/common/utils/ChunkedBytesStream.java:
##########
@@ -290,25 +290,27 @@ public long skip(long toSkip) throws IOException {
// Skip bytes stored in intermediate buffer first
int avail = count - pos;
- long bytesSkipped = (avail < remaining) ? avail : remaining;
+ int bytesSkipped = Math.min(avail, (int) remaining);
Review Comment:
I am concerned about cases where `remaining` is outside the range of
representable `int` values, let's say `remaining = Integer.MIN_VALUE + 1000`.
When down casting in cases of overflow (i.e. in `Math.min(avail, (int)
remaining)`), the behaviour is implementation defined and numeric value for
`(int)remaining` may end up leading to a negative value numeric int.
Calculating min for this negative value with avail will lead to bytesSkipped as
negative.
In the implementation I suggested, the result of `Math.min((long) avail,
remaining)` is guaranteed to fit in `int` because it's upper bound by `avail`.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
