Skip to content

FixedCompositeByteBuf should allow to access memoryAddress / array wh…#7773

Merged
normanmaurer merged 1 commit into
4.1from
fixed_buffer_memoryaddress
Mar 13, 2018
Merged

FixedCompositeByteBuf should allow to access memoryAddress / array wh…#7773
normanmaurer merged 1 commit into
4.1from
fixed_buffer_memoryaddress

Conversation

@normanmaurer
Copy link
Copy Markdown
Member

…en wrap a single buffer.

Motivation:

We should allow to access the memoryAddress / array of the FixedCompositeByteBuf when it only wraps a single ByteBuf. We do the same for CompositeByteBuf.

Modifications:

  • Check how many buffers FixedCompositeByteBuf wraps and depending on it delegate the access to the memoryAddress / array
  • Add unit tests.

Result:

Fixes [#7752].

…en wrap a single buffer.

Motivation:

We should allow to access the memoryAddress / array of the FixedCompositeByteBuf when it only wraps a single ByteBuf. We do the same for CompositeByteBuf.

Modifications:

- Check how many buffers FixedCompositeByteBuf wraps and depending on it delegate the access to the memoryAddress / array
- Add unit tests.

Result:

Fixes [#7752].
@normanmaurer
Copy link
Copy Markdown
Member Author

Also @ldaley

Copy link
Copy Markdown
Member

@carl-mastrangelo carl-mastrangelo left a comment

Choose a reason for hiding this comment

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

LGTM, looks pretty straightforward

@normanmaurer normanmaurer merged commit bd772d1 into 4.1 Mar 13, 2018
@normanmaurer normanmaurer added this to the 4.1.23.Final milestone Mar 13, 2018
@normanmaurer normanmaurer self-assigned this Mar 13, 2018
@carl-mastrangelo
Copy link
Copy Markdown
Member

Was the branch fixed_buffer_memoryaddress supposed to stay around?

@normanmaurer normanmaurer deleted the fixed_buffer_memoryaddress branch March 15, 2018 16:34
@normanmaurer
Copy link
Copy Markdown
Member Author

@carl-mastrangelo nope... deleted. Thanks

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.

2 participants