Skip to content

nanocoap_link_format: fix off-by-one error#19307

Merged
bors[bot] merged 1 commit intoRIOT-OS:masterfrom
benpicco:nanocoap/link_format-1
Feb 23, 2023
Merged

nanocoap_link_format: fix off-by-one error#19307
bors[bot] merged 1 commit intoRIOT-OS:masterfrom
benpicco:nanocoap/link_format-1

Conversation

@benpicco
Copy link
Copy Markdown
Contributor

@benpicco benpicco commented Feb 23, 2023 •
edited
Loading

Contribution description

We set the byte at the end postition to 0, but buffer[sizeof(buffer)] is actually outside of buffer, we have to use sizeof(buffer) - 1.

Testing procedure

Thanks to @Teufelchen1 for spotting this!

Resource listing still works

> ncget coap://coap.me/
/test
/validate
/hello
/blåbærsyltetøy
/sink
/separate
/large
/secret
/broken
/weird33
/weird44
/weird55
/weird333
/weird3333
/weird33333
/123412341234123412341234
/location-query
/create1
/large-update
/large-create
/query
/seg1
/path
/location1
/multi-format
/3
/4
/5

Issues/PRs references

@github-actions github-actions bot added Area: CoAP Area: Constrained Application Protocol implementations Area: network Area: Networking Area: sys Area: System labels Feb 23, 2023
@benpicco benpicco added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) 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 and removed Area: network Area: Networking Area: CoAP Area: Constrained Application Protocol implementations Area: sys Area: System labels Feb 23, 2023
@benpicco benpicco requested a review from maribu February 23, 2023 15:39
miri64
miri64 approved these changes Feb 23, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@miri64 miri64 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

ACK. Explanation is sensible and testing looks sane.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Feb 23, 2023

bors merge

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Feb 23, 2023

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@riot-ci
Copy link
Copy Markdown

riot-ci commented Feb 23, 2023

Murdock results

✔️ PASSED

7468c62 nanocoap_link_format: fix off-by-one error

Success Failures Total Runtime
6865 0 6865 10m:32s

Artifacts

@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Feb 23, 2023

bors merge

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Feb 23, 2023

Already running a review

bors bot added a commit that referenced this pull request Feb 23, 2023
@bors @maribu @benpicco
Merge #19292 #19307
04aee51 
19292: sys/phydat: Fix unit confusion r=benpicco a=maribu

### Contribution description

Previously, `UNIT_G` was used for g-force with the correct symbol `g`, `UNIT_GR` for gram (as in kilogram) with the incorrect symbol `G` (which would be correct for Gauss), and `UNIT_GS` for Gauss with symbol `Gs` (which is an alternative correct symbol).

To avoid confusion between G-Force, Gauss, and Gram the units have been renamed to `UNIT_G_FORCE`, `UNIT_GRAM`, and `UNIT_GAUSS`. In addition, gram now uses the correct symbol `g`; which sadly is the same as for g-force. But usually there is enough context to tell them apart.

### Testing procedure

Green CI

### Issues/PRs references

None

19307: nanocoap_link_format: fix off-by-one error r=miri64 a=benpicco



Co-authored-by: Marian Buschsieweke <[email protected]>
Co-authored-by: Benjamin Valentin <[email protected]>
@bors
Copy link
Copy Markdown
Contributor

bors bot commented Feb 23, 2023

This PR was included in a batch that was canceled, it will be automatically retried

@miri64
Copy link
Copy Markdown
Member

miri64 commented Feb 23, 2023

bors merge

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Feb 23, 2023

Already running a review

bors bot added a commit that referenced this pull request Feb 23, 2023
@bors @maribu @benpicco
Merge #19292 #19307
0bf22a8 
19292: sys/phydat: Fix unit confusion r=miri64 a=maribu

### Contribution description

Previously, `UNIT_G` was used for g-force with the correct symbol `g`, `UNIT_GR` for gram (as in kilogram) with the incorrect symbol `G` (which would be correct for Gauss), and `UNIT_GS` for Gauss with symbol `Gs` (which is an alternative correct symbol).

To avoid confusion between G-Force, Gauss, and Gram the units have been renamed to `UNIT_G_FORCE`, `UNIT_GRAM`, and `UNIT_GAUSS`. In addition, gram now uses the correct symbol `g`; which sadly is the same as for g-force. But usually there is enough context to tell them apart.

### Testing procedure

Green CI

### Issues/PRs references

None

19307: nanocoap_link_format: fix off-by-one error r=miri64 a=benpicco



Co-authored-by: Marian Buschsieweke <[email protected]>
Co-authored-by: Benjamin Valentin <[email protected]>
@bors
Copy link
Copy Markdown
Contributor

bors bot commented Feb 23, 2023

This PR was included in a batch that was canceled, it will be automatically retried

@miri64
Copy link
Copy Markdown
Member

miri64 commented Feb 23, 2023

bors cancel

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Feb 23, 2023

Canceled.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Feb 23, 2023

bors merge

@miri64
Copy link
Copy Markdown
Member

miri64 commented Feb 23, 2023

bors cancel
bors merge

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Feb 23, 2023

Canceled.

bors bot added a commit that referenced this pull request Feb 23, 2023
@bors @maribu @benpicco
Merge #19292 #19307
df35e96 
19292: sys/phydat: Fix unit confusion r=miri64 a=maribu

### Contribution description

Previously, `UNIT_G` was used for g-force with the correct symbol `g`, `UNIT_GR` for gram (as in kilogram) with the incorrect symbol `G` (which would be correct for Gauss), and `UNIT_GS` for Gauss with symbol `Gs` (which is an alternative correct symbol).

To avoid confusion between G-Force, Gauss, and Gram the units have been renamed to `UNIT_G_FORCE`, `UNIT_GRAM`, and `UNIT_GAUSS`. In addition, gram now uses the correct symbol `g`; which sadly is the same as for g-force. But usually there is enough context to tell them apart.

### Testing procedure

Green CI

### Issues/PRs references

None

19307: nanocoap_link_format: fix off-by-one error r=miri64 a=benpicco



Co-authored-by: Marian Buschsieweke <[email protected]>
Co-authored-by: Benjamin Valentin <[email protected]>
@bors
Copy link
Copy Markdown
Contributor

bors bot commented Feb 23, 2023

Build succeeded:

@bors bors bot merged commit 4809b68 into RIOT-OS:master Feb 23, 2023
@benpicco benpicco deleted the nanocoap/link_format-1 branch February 23, 2023 23:58
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miri64 miri64 miri64 approved these changes

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

@cgundogan cgundogan Awaiting requested review from cgundogan

@maribu maribu Awaiting requested review from maribu

Assignees

No one assigned

Labels

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

Milestone

Release 2023.04

Development

Successfully merging this pull request may close these issues.

4 participants

@benpicco @miri64 @riot-ci @MrKevinWeiss