[ 
https://issues.apache.org/jira/browse/CASSANDRA-12381?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15408044#comment-15408044
 ] 

Sylvain Lebresne commented on CASSANDRA-12381:
----------------------------------------------

bq. Strictly, we have some misuses (assuming !hasArray() == direct buffer) for 
example in 
org.apache.cassandra.io.util.BufferedDataOutputStreamPlus#write(java.nio.ByteBuffer)

You have be careful with logic :). If we get precise, the clearly incorrect 
assumption is that {{isDirect()}} implies {{!hasArray()}} (the javadoc is very 
explicit on that).

However, It does sound kind of implied (by the doc on {{allocate()}}) that non 
direct buffers do always have a backed buffer, and so that {{!hasArray()}} does 
imply {{isDirect()}}, which is what we rely on here. To be fair, the one thing 
I haven't found is a place that explicitly said that non-direct buffer can 
*only* be built by either {{allocate()}} or {{wrap()}} (or some 
slicing/duplicate on a non-direct one), so in theory I wouldn't mind updating 
that part of the code, but I genuinely don't know what you'd do of a buffer 
that doesn't have a backing array but isn't direct either. That is, if you're 
only changing {{hasArray()}} to {{!isDirect()}} in that code, you're really 
making the exact same assumption the code already does, but it's less explicit 
imo. Overall, this code clearly states the assumption it does make, so I think 
it's fine.

Again, I'm totally fine fixing the {{isDirect()}} => {{!hasArray()}} assumption 
if we ever do it, but that's not an example of that.

> Verify use of ByteBuffer.hasArray() in whole code base
> ------------------------------------------------------
>
>                 Key: CASSANDRA-12381
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-12381
>             Project: Cassandra
>          Issue Type: Bug
>            Reporter: Robert Stupp
>            Assignee: Robert Stupp
>            Priority: Minor
>             Fix For: 3.0.x
>
>
> As [noted 
> here|https://docs.oracle.com/javase/8/docs/api/java/nio/ByteBuffer.html#allocateDirect-int-],
>  a direct {{ByteBuffer}} can have a backing array.
> If a direct {{ByteBuffer}} has a backing array, we should not make any 
> assumption on how it is actually used.
> This ticket is about to check whether uses of {{ByteBuffer.hasArray()}} need 
> to be replaced with {{ByteBuffer.isDirect()}}.
> (With CASSANDRA-11870 however, there is no way that such a direct 
> {{ByteBuffer}} has a backing array.)



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to