Skip to content

Conversation

@anordal
Copy link
Contributor

@anordal anordal commented Feb 25, 2025

Let's build unittests with certain warnings enabled and turned into errors.
This will make them fail in CI and also make them look like errors in editors that use a language server.

In particular, -Werror=uninitialized (make compilation fail when all control paths use a variable uninitialized).
The first commit is devoted to fixing these.

Motivation: Most of the crashing tests I fixed in #1151 were caused by reading uninitialized memory. As I suggested there, the kinds of errors I had to fix could have been prevented by enabling certain warnings as errors.

Checklist:

  • I have tested my changes, as in running the unittests. The only changes are to unittests and the CMake file that builds them.
  • Coverage: Maintains 100% branch coverage. I removed some tests that didn't contribute to this.

Related Issue

This is a continuation of #1151.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@anordal anordal requested a review from a team as a code owner February 25, 2025 23:20
@xuelix
Copy link
Member

xuelix commented Feb 26, 2025

Thanks for the effort, we'll have the team to review the change.

Copy link
Member

@ActoryOu ActoryOu left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!
I'm good with most of changes. And here are some nitpicky suggestions from me.

This commit also fixes 4 of these warnings, that occured once each,
namely -Werror={switch,memset-elt-size,comment,unused-value}.
@tony-josi-aws
Copy link
Member

Thanks, @anordal, for your contribution to FreeRTOS+TCP.

@tony-josi-aws tony-josi-aws merged commit da065eb into FreeRTOS:main Mar 12, 2025
10 checks passed
@anordal anordal deleted the unittest-warnings branch March 12, 2025 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants