Skip to content

sys/bitfield: don't touch unrelated bits in bf_{set, clear}_all() #19400

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

Merged
merged 2 commits into from
Mar 20, 2023

Conversation

benpicco
Copy link
Contributor

Contribution description

When I added those functions I thought there was no use-case for not touching the remaining bits of the last byte of the bitfield (e.g. in a bitfield of size 5 a bf_clear_all() would set all 8 bits of the bitfield-byte to 0).

But it turns out there is a use case for only clearing/setting part of a bitfield, so make the bf_{set, clear}_all() functions actually honor the proper size they are called with.

Testing procedure

A new test case has been added to the unit test to check if no unrelated bits are touched.
This test case will fail on master.

Issues/PRs references

@benpicco benpicco requested a review from miri64 as a code owner March 16, 2023 23:11
@benpicco benpicco requested a review from maribu March 16, 2023 23:11
@github-actions github-actions bot added Area: sys Area: System Area: tests Area: tests and testing framework labels Mar 16, 2023
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Mar 16, 2023
@benpicco benpicco changed the title sys/bitfield: don't set unrelated bits in bf_{set, clear}_all() sys/bitfield: don't touch unrelated bits in bf_{set, clear}_all() Mar 16, 2023
@riot-ci
Copy link

riot-ci commented Mar 16, 2023

Murdock results

✔️ PASSED

eb9fbfb tests/unittests: add test for bf_clear_all()

Success Failures Total Runtime
6882 0 6882 10m:14s

Artifacts

@benpicco benpicco requested a review from kaspar030 March 20, 2023 10:45
Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

ACK.

@kaspar030
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Mar 20, 2023

Build succeeded:

@bors bors bot merged commit 189d1c5 into RIOT-OS:master Mar 20, 2023
@benpicco benpicco deleted the bf_clear_all branch March 20, 2023 13:40
@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
Labels
Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants