Github user aweisberg commented on a diff in the pull request:

    https://github.com/apache/cassandra/pull/239#discussion_r200704746
  
    --- Diff: 
src/java/org/apache/cassandra/net/async/RebufferingByteBufDataInputPlus.java ---
    @@ -249,4 +253,50 @@ public ByteBufAllocator getAllocator()
         {
             return channelConfig.getAllocator();
         }
    +
    +    /**
    +     * Consumes bytes in the stream until the given length
    +     *
    +     * @param writer
    +     * @param len
    +     * @return
    +     * @throws IOException
    +     */
    +    public long consumeUntil(BufferedDataOutputStreamPlus writer, long 
len) throws IOException
    +    {
    +        long copied = 0; // number of bytes copied
    +        while (copied < len)
    +        {
    +            int position = buffer.position();
    +            int remaining = buffer.remaining();
    +            if (remaining == 0)
    +            {
    +                try
    +                {
    +                    reBuffer();
    +                } catch (EOFException e)
    +                {
    +                    throw new EOFException("EOF after " + copied + " bytes 
out of " + len);
    +                }
    +                position = buffer.position();
    +                remaining = buffer.remaining();
    +                if (remaining == 0)
    +                    return copied == 0 ? -1 : copied;
    +            }
    +
    +            int toCopy = (int) Math.min(len - copied, remaining);
    +
    +            ByteBuffer dup = buffer.duplicate();
    --- End diff --
    
    I don't see why you would store position on the stack outside of the buffer 
it really just seems like an opportunity to get it wrong by not going direct to 
the buffer. Same with remaining it's only ever used one after it is ever stored 
on the stack so why store it on the stack at all?
    I hate to keep bikeshedding this, but I don't even see why you need the dup 
or to manually update the position? 
    
    ```
    if (buffer.remaining() == 0)
    {
        try 
       {
            reBuffer();
        }
        catch (EOFException e)
        {
            throw new EOFException("EOF after " + copied + " bytes out of " + 
len);
        }
        if (buffer.remaining() == 0)
        {
            return copied == 0 ? -1 : copied;
        }
    }
    
    int originalLimit = buffer.limit();
    buffer.limit(Math.min(len - copied, remaining);
    int written = writer.applyToChannel(c -> c.write(buffer));
    buffer.limit(originalLimit);
    copied += written;
    ```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to