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