Skip to content

Add ByteBuffer input support again - #377

Closed
miniway wants to merge 1 commit into
msgpack:developfrom
miniway:develop
Closed

Add ByteBuffer input support again#377
miniway wants to merge 1 commit into
msgpack:developfrom
miniway:develop

Conversation

@miniway

@miniway miniway commented Jul 27, 2016

Copy link
Copy Markdown
Contributor

No description provided.


public ByteBufferInput(ByteBuffer input)
{
this.input = checkNotNull(input, "input ByteBuffer is null");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is better to call slice() as following to avoid this unexpected scenario:

  • An user creates a new ByteBuffer and fill some data there
  • The user creates a new ByteBufferInput with the ByteBuffer
  • the user changes position of the ByteBuffer! That might be by another thread.
  • ByteBufferInput is affected the change of position, unexpectedly
        this.input = checkNotNull(input, "input ByteBuffer is null").slice();

@frsyuki

frsyuki commented Jul 27, 2016

Copy link
Copy Markdown
Member

@miniway I wrote code for above comments at #378.
@xerial could you also review #378? Especially around improvements of exception handling and documents.

@xerial

xerial commented Jul 27, 2016

Copy link
Copy Markdown
Member

sure. will do

@miniway

miniway commented Jul 28, 2016

Copy link
Copy Markdown
Contributor Author

I've updated the PR based on the comments rollbacked as we have #378

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants