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

[collectd 6] Fix some gcc warnings with more strict checks #3970

Merged
merged 8 commits into from
Jun 8, 2022

Conversation

eero-t
Copy link
Contributor

@eero-t eero-t commented Feb 4, 2022

ChangeLog: Build: Fix some gcc warnings with more strict checks

Fixes are based on "-O3 -Werror -Wall -Wextra -Wformat-security" output, and partly shared with the v5 fixes in #3917:

  • Properly initialize complex struct
  • Fix signed vs unsigned comparison warnings
  • Correct "write_prometheus" comment and fix metric family freeing in a loop
  • Remove unused functions and function arguments, and when latter cannot be removed, tell compiler that they're not used

@eero-t eero-t requested review from a team as code owners February 4, 2022 13:03
@collectd-bot collectd-bot added this to the 6.0 milestone Feb 4, 2022
@eero-t
Copy link
Contributor Author

eero-t commented Feb 4, 2022

If you want some of the commits to be moved to another PR or dropped, just tell which ones.

PS. If somebody wants to do similar thing for rest of collectd code, I've filed #3969 for some additional issues in plugins that this does not handle.

@eero-t
Copy link
Contributor Author

eero-t commented Feb 4, 2022

debian_default_toolchain container:collectd/ci:trusty_amd64 Failing

@mrunge could you apply v5 fix for this CI issue also to v6 branch?

@mrunge
Copy link
Member

mrunge commented Feb 8, 2022

debian_default_toolchain container:collectd/ci:trusty_amd64 Failing

@mrunge could you apply v5 fix for this CI issue also to v6 branch?

Yes, of course.

@mrunge
Copy link
Member

mrunge commented Feb 8, 2022

Proposed the change here: #3972

"format_stackdriver" was migrated over year ago.
* Properly initialize complex struct
* Fix signed vs unsigned comparisons
* Tell compiler which args are expected to be unused

Based on "-O3 -Werror -Wall -Wextra -Wformat-security" output.
* Fix unused arguments reported by:
  "-O3 -Werror -Wall -Wextra -Wformat-security"
* Fix obsolete comment to match MHD docs:
  https://www.gnu.org/software/libmicrohttpd/ ("Queueing responses" section)
  https://git.gnunet.org/libmicrohttpd.git/tree/src/include/microhttpd.h#n2398
* Fix use after free reported by Klocwork, "prom_fam" cannot be used
  after it's been freed
Based on "-O3 -Werror -Wall -Wextra -Wformat-security" output.
Based on "-O3 -Werror -Wall -Wextra -Wformat-security" output.
Based on "-O3 -Werror -Wall -Wextra -Wformat-security" output.
Apparently CI has changed since this code was added to collectd.
@eero-t eero-t force-pushed the collectd-6.0-gcc-warnings branch from 5139dc7 to 8fb7c6b Compare May 30, 2022 14:27
@eero-t
Copy link
Contributor Author

eero-t commented May 30, 2022

Rebased to latest v6 branch to get tests passing. It might be better to merge v5 version of these fixes (#3917) first to make sure these branches do not get out of sync.

Copy link
Member

@mrunge mrunge left a comment

Choose a reason for hiding this comment

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

thank you, looks good

@mrunge mrunge merged commit 62221e6 into collectd:collectd-6.0 Jun 8, 2022
@eero-t eero-t deleted the collectd-6.0-gcc-warnings branch July 20, 2022 11:22
@eero-t eero-t added the Fix A pull request fixing a bug label Jan 15, 2024
@eero-t
Copy link
Contributor Author

eero-t commented Jan 15, 2024

Used "Fix" label because some of the warnings were actually visible to users as build errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Fix A pull request fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants