Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[backport] drivers/w5100: Fixed netdev_driver_t::recv() API #10973

Merged
merged 2 commits into from
Feb 8, 2019
Merged

[backport] drivers/w5100: Fixed netdev_driver_t::recv() API #10973

merged 2 commits into from
Feb 8, 2019

Conversation

maribu
Copy link
Member

@maribu maribu commented Feb 8, 2019

Backport of #10412

Contribution description

The current implementation of the netdev_driver_t::recv() API in the W5100 driver does not comply with documentation:

  • The drop feature of recv() is not implemented
  • In case of a too small buffer no error is returned and the frame is silently truncated. The API mandates returning -ENOBUFS

Testing procedure

It would be nice to add a test application for this, as many drivers are affected...

Issues/PRs references

#10410, #10413

maribu added 2 commits February 8, 2019 12:37
@maribu
drivers/w5100: Added missing drop implementation
2b3a0d2 
The netdev_driver_t::recv implementation of the w5100 does not provide the
drop feature. This commit adds it.

Fixes: RIOT-OS#10410
@maribu
drivers/w5100: Fix recv() with too small buffer
86e48b4 
If netdev_driver_t::recv() is called and the provided buffer is smaller than
the frame then `-ENOBUFS` should be returned, the frame should be dropped, and
no data of the frame should be returned.

Addresses: RIOT-OS#10413
@maribu maribu added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master labels Feb 8, 2019
@maribu maribu requested a review from aabadie February 8, 2019 11:40
aabadie
aabadie approved these changes Feb 8, 2019
View reviewed changes
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

I verified the changes are the same compared to original PR.

ACK!

@aabadie
Copy link
Contributor

aabadie commented Feb 8, 2019

and go

@aabadie aabadie merged commit ca96dae into RIOT-OS:2019.01-branch Feb 8, 2019
@maribu maribu deleted the w5100_backport branch February 8, 2019 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aabadie aabadie aabadie approved these changes

Assignees
No one assigned
Labels
Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@maribu @aabadie