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

tests/unittests/tests-hashes: fix wrong string length #20165

Merged
merged 1 commit into from
Dec 11, 2023

Conversation

fzi-haxel
Copy link
Contributor

@fzi-haxel fzi-haxel commented Dec 11, 2023

Contribution description

Incorrect use of sizeof for string length in test test_hashes_sha512_hash_update_twice.

static const char *teststring is a pointer, therefore sizeof(teststring) returns the pointer size instead of the intended string length. This PR uses strlen instead of sizeof to fix this. Alternatively, we could change the type of the string to static const char teststring[].

Note the subtle difference between sizeof(char[]) and strlen: The former includes the null terminator, while strlen does not. This should not change the test behavior.

Edit:
This PR changes the type to static const char teststring[] so that sizeof returns the string length (including the null terminator).

Testing procedure

Compile and run unittests.

Note for 32-bit architectures: The test also passed before, but only used 4 and 3+1 bytes of the string.

@fzi-haxel fzi-haxel requested a review from miri64 as a code owner December 11, 2023 10:46
@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Dec 11, 2023
@fzi-haxel fzi-haxel force-pushed the pr/tests-hashes-sha512-fix branch from f1efa99 to 8db9811 Compare December 11, 2023 13:10
@fzi-haxel fzi-haxel force-pushed the pr/tests-hashes-sha512-fix branch from 8db9811 to 4a1d07a Compare December 11, 2023 13:16
fzi-haxel added a commit to fzi-haxel/RIOT that referenced this pull request Dec 11, 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 labels Dec 11, 2023
@riot-ci
Copy link

riot-ci commented Dec 11, 2023

Murdock results

✔️ PASSED

4a1d07a tests/unittests/tests-hashes: fixes wrong string length

Success Failures Total Runtime
13 0 13 01m:23s

Artifacts

@benpicco benpicco added this pull request to the merge queue Dec 11, 2023
Merged via the queue into RIOT-OS:master with commit fd3d92f Dec 11, 2023
29 checks passed
fzi-haxel added a commit to fzi-haxel/RIOT that referenced this pull request Dec 12, 2023
OlegHahm pushed a commit to OlegHahm/RIOT that referenced this pull request Dec 13, 2023
OlegHahm pushed a commit to OlegHahm/RIOT that referenced this pull request Dec 13, 2023
OlegHahm pushed a commit to OlegHahm/RIOT that referenced this pull request Dec 21, 2023
@fzi-haxel fzi-haxel deleted the pr/tests-hashes-sha512-fix branch January 8, 2024 13:03
@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: 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 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.

4 participants