Skip to content

Fix epoll spliceTo file descriptor with offset#9369

Merged
normanmaurer merged 1 commit intonetty:4.1from
njhill:splice-offset
Jul 16, 2019
Merged

Fix epoll spliceTo file descriptor with offset#9369
normanmaurer merged 1 commit intonetty:4.1from
njhill:splice-offset

Conversation

@njhill
Copy link
Copy Markdown
Member

@njhill njhill commented Jul 15, 2019

Motivation

The AbstractEpollStreamChannel::spliceTo(FileDescriptor, ...) methods take an offset parameter but this was effectively ignored due to what looks like a typo in the corresponding JNI function impl. Instead it would always use the file's own native offset.

Modification

  • Fix typo in netty_epoll_native_splice0() and offset accounting in AbstractEpollStreamChannel::SpliceFdTask.
  • Modify unit test to include an invocation of the public spliceTo method using non-zero offset.

Result

spliceTo FD methods work as expected when an offset is provided. Note this does change the prior behaviour in that the file's native offset was previously modified but will now be unchanged - we could make a further modification to preserve that though if necessary.

Motivation

The AbstractEpollStreamChannel::spliceTo(FileDescriptor, ...) methods
take an offset parameter but this was effectively ignored due to what
looks like a typo in the corresponding JNI function impl. Instead it
would always use the file's own native offset.

Modification

- Fix typo in netty_epoll_native_splice0() and offset accounting in
AbstractEpollStreamChannel::SpliceFdTask.
- Modify unit test to include an invocation of the public spliceTo
method using non-zero offset.

Result

spliceTo FD methods work as expected when an offset is provided.
@njhill njhill added the defect label Jul 15, 2019
@njhill njhill requested a review from normanmaurer July 15, 2019 06:12
@netty-bot
Copy link
Copy Markdown

Can one of the admins verify this patch?

@normanmaurer
Copy link
Copy Markdown
Member

@netty-bot test this please

@normanmaurer normanmaurer added this to the 4.1.38.Final milestone Jul 16, 2019
@normanmaurer normanmaurer merged commit 1748352 into netty:4.1 Jul 16, 2019
normanmaurer pushed a commit that referenced this pull request Jul 16, 2019
Motivation

The AbstractEpollStreamChannel::spliceTo(FileDescriptor, ...) methods
take an offset parameter but this was effectively ignored due to what
looks like a typo in the corresponding JNI function impl. Instead it
would always use the file's own native offset.

Modification

- Fix typo in netty_epoll_native_splice0() and offset accounting in
AbstractEpollStreamChannel::SpliceFdTask.
- Modify unit test to include an invocation of the public spliceTo
method using non-zero offset.

Result

spliceTo FD methods work as expected when an offset is provided.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants