Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4926314: Optimize Reader.read(CharBuffer) #1915

Open
wants to merge 15 commits into
base: master
from
@@ -202,6 +202,7 @@ public int read(CharBuffer target) throws IOException {
do {
// read to EOF which may read more or less than buffer size
if ((n = read(cbuf)) > 0) {
target.put(cbuf, 0, n);
nread += n;
remaining -= n;

This comment has been minimized.

@plevart

plevart Jan 18, 2021
Contributor

Wouldn't there be a possibility for target.put(cbuf, 0, n) to throw BufferOverflowException ?
For example:

  • there's room (remaining) for TRANSFER_BUFFER_SIZE + 1 characters in target
  • cbuff is sized to TRANSFER_BUFFER_SIZE
  • 1st iteration of do loop transfers TRANSFER_BUFFER_SIZE charasters (remaining == 1)
  • 2nd iteration reads > 1 (up to TRANSFER_BUFFER_SIZE) characters
  • target.put throws BufferOverflowException

You have to limit the amount read in each iteration to be Math.min(remaining, cbuf.length)

This comment has been minimized.

@marschall

marschall Jan 19, 2021
Author Contributor

You're correct. I need to expand the unit tests.

This comment has been minimized.

@marschall

marschall Jan 26, 2021
Author Contributor

Fixed

}
ProTip! Use n and p to navigate between commits in a pull request.