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.

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

nanocoap_sock: only abort nanocoap_sock_get_blockwise() on negative error #18037

Merged
merged 1 commit into from
Apr 29, 2022

Conversation

benpicco
Copy link
Contributor

Contribution description

Some user callbacks might just return the result of some other operation that returns written bytes or negative error.

Let's not break those, only consider negative callback returns an error.

Testing procedure

I discovered this when rebasing #17937 onto master.
Only the first block of a file would be written.
This is because the callback just returns the result of vfs_write(). This used to work fine (because nanocoap_sock_request_cb() would change the return code based on the CoAP response code), now it is handed through to the user, breaking nanocoap_sock_get_blockwise().

Issues/PRs references

follow-up to #17950

…rror

Some user callbacks might just return the result of some other operation
that returns written bytes or negative error.

Let's not break those, only consider negative callback returns an error.
@benpicco benpicco added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Apr 29, 2022
@github-actions github-actions bot added Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System labels Apr 29, 2022
@benpicco benpicco added Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 29, 2022
@benpicco benpicco requested review from chrysn, fjmolinas and maribu April 29, 2022 19:35
@benpicco benpicco enabled auto-merge April 29, 2022 20:17
@benpicco benpicco merged commit 164220f into RIOT-OS:master Apr 29, 2022
@benpicco benpicco deleted the nanocoap_sock-error_return branch April 29, 2022 22:32
@chrysn chrysn added this to the Release 2022.07 milestone Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants