Skip to content

Conversation

Teufelchen1
Copy link
Contributor

@Teufelchen1 Teufelchen1 commented Dec 19, 2023

Contribution description

Hello 🦙

this adds a few new test cases to the tests-fmt unit tests.
The changes only apply to these functions:

  • fmt_is_digit()
  • fmt_is_upper()
  • fmt_is_number()
  • fmt_byte_hex()
  • fmt_bytes_hex()
  • fmt_bytes_hex_reverse()
  • fmt_hex_byte()
  • fmt_hex_bytes()

I found the behavior of fmt_bytes_hex_reverse() to be not conform with the API description. In this case, a null-pointer de-reference could be triggered while complying with the API, by passing a NULL pointer as the output buffer. A search through the code base returned only few usage of this function and none of it was affected. I applied a fix and also moved the function closer to its relative fmt_bytes_hex(), thereby mimicking the order present in fmt.h. As an extra, I refactored the function to look even more similar to its relative.

In addition, one statement in the API of fmt_hex_byte() in fmt.h was clarified and a missing statement was added.

Thanks, 🐮

Testing procedure

make -C tests/unittests/ tests-fmt test

Issues/PRs references

Most tests touched were introduced in #8343

@github-actions github-actions bot added Area: tests Area: tests and testing framework Area: sys Area: System labels Dec 19, 2023
TEST_ASSERT_EQUAL_INT(0x00, byte);

memcpy(hex, "12", 2);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure about the purpose of these memcpy, can someone enlighten me?

@Teufelchen1 Teufelchen1 added the CI: low priority If set, builds of this PR will be queued behind others label Dec 19, 2023
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 21, 2023
@riot-ci
Copy link

riot-ci commented Dec 21, 2023

Murdock results

✔️ PASSED

4a62b9f tests: Slightly increase coverage of fmt unittests

Success Failures Total Runtime
8097 0 8098 10m:13s

Artifacts

@benpicco benpicco added this pull request to the merge queue Jan 2, 2024
Merged via the queue into RIOT-OS:master with commit 59eb017 Jan 2, 2024
@MrKevinWeiss MrKevinWeiss added this to the Release 2024.01 milestone Feb 7, 2024
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: low priority If set, builds of this PR will be queued behind others CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants