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

ASF GitHub Bot commented on FLINK-7516:
---------------------------------------

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

    https://github.com/apache/flink/pull/4593#discussion_r148204666
  
    --- Diff: 
flink-core/src/main/java/org/apache/flink/core/memory/HybridMemorySegment.java 
---
    @@ -306,6 +307,9 @@ public final void get(int offset, ByteBuffer target, 
int numBytes) {
                if ((offset | numBytes | (offset + numBytes)) < 0) {
                        throw new IndexOutOfBoundsException();
                }
    +           if (target.isReadOnly()) {
    --- End diff --
    
    Isn't this check redundant? Shouldn't the `ByteBuffer`s validate it on 
their own like `DirectByteBufferR` do? Putting this check here, it complicates 
the code and we have to pay for it on each call, even on happy path (cost 
should be very tiny). 
    
    Maybe it should be put only in `if (target.isDirect())` branch to check 
`read-only` only before unsafe copy?


> HybridMemorySegment: do not allow copies into a read-only ByteBuffer
> --------------------------------------------------------------------
>
>                 Key: FLINK-7516
>                 URL: https://issues.apache.org/jira/browse/FLINK-7516
>             Project: Flink
>          Issue Type: Sub-task
>          Components: Core, Network
>    Affects Versions: 1.4.0
>            Reporter: Nico Kruber
>            Assignee: Nico Kruber
>            Priority: Major
>
> {{HybridMemorySegment#get(int, ByteBuffer, int)}} allows writing into a 
> read-only {{ByteBuffer}} but this operation should be forbidden.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to