[
https://issues.apache.org/jira/browse/ZOOKEEPER-1843?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13981507#comment-13981507
]
Michi Mutsuzaki commented on ZOOKEEPER-1843:
--------------------------------------------
Thank you for the patch Bill. It looks great overall with good set of tests. I
just have 2 minor comments:
- Add a license header to ByteBufferInputStreamTest.java. The license header
should look something like this:
https://github.com/apache/zookeeper/blob/trunk/src/java/test/org/apache/zookeeper/server/CRCTest.java
- Wrap long lines around 80 characters.
> Oddity in ByteBufferInputStream skip
> ------------------------------------
>
> Key: ZOOKEEPER-1843
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1843
> Project: ZooKeeper
> Issue Type: Bug
> Reporter: Justin SB
> Assignee: Bill Havanki
> Priority: Minor
> Fix For: 3.5.0
>
> Attachments: ZOOKEEPER-1843.patch
>
>
> I was reading ByteBufferInputStream.java; here is the skip method:
> public long skip(long n) throws IOException {
> long newPos = bb.position() + n;
> if (newPos > bb.remaining()) {
> n = bb.remaining();
> }
> bb.position(bb.position() + (int) n);
> return n;
> }
> The first two lines look wrong; we compare a "point" (position) to a
> "distance" (remaining). I think the test should be if (newPos >= bb.limit()).
> Or more simply:
> public long skip(long n) throws IOException {
> int remaining = buffer.remaining();
> if (n > remaining) {
> n = remaining;
> }
> buffer.position(buffer.position() + (int) n);
> return n;
> }
--
This message was sent by Atlassian JIRA
(v6.2#6252)
