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

CDRIVER-4484 Address -Wformat and -Wstrict-prototypes warnings #1120

Merged
merged 4 commits into from
Oct 13, 2022

Conversation

eramongodb
Copy link
Contributor

Description

This PR resolves CDRIVER-4484 by addressing -Wformat=2 and -Wstrict-prototypes warnings.

Warnings were detected using GCC 12.1.0 and Clang 10.0.0 on Ubuntu 20.04 and GCC 12.1.0 (mingw-w64) on Windows with ENABLE_CLIENT_SIDE_ENCRYPTION=(OFF|ON) and ENABLE_TRACING=(OFF|ON).

Copy link
Collaborator

@vector-of-bool vector-of-bool left a comment

Choose a reason for hiding this comment

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

LGTM, but can we add these warnings to somewhere that they will appear again? i.e. maintainer-flags or MongoC-Warnings.cmake?

@eramongodb
Copy link
Contributor Author

can we add these warnings to somewhere that they will appear again?

-Wformat=1 is already enabled implicity via -Wall according to GCC Options documentation, which is already enabled when building on Linux and MacOS distros (ENABLE_MAINTAINER_FLAGS=ON -> maintainer-flags.txt). I do not think any of the instances that were reported required -Wformat=2.

Most of these warnings have escaped noticed due to requiring:

  1. ENABLE_TRACING=ON, which is only done for the compile-tracing task run on Ubuntu 18.04), and
  2. compiling Windows code paths with GCC (i.e. MinGW), as MSVC does not have -Wformat.

This may be an argument for adding the compile-tracing task to more variants such as Windows. I will consider doing so when working on CDRIVER-3620.

As for -Wstrict-prototypes, it seems to be a hit-or-miss when testing with Clang. Testing with GCC uncovered a few more instances. Still, I think it is safe to add to the set of maintainer flags. Filed CDRIVER-4504 as followup work.

@eramongodb eramongodb merged commit fd80583 into mongodb:master Oct 13, 2022
@eramongodb eramongodb deleted the cdriver-4484 branch October 13, 2022 19:46
@jeroen
Copy link
Contributor

jeroen commented Oct 22, 2022

Which release would this be included in? I noticed it hasn't been included in 1.23.1 and 1.22.2

@eramongodb
Copy link
Contributor Author

eramongodb commented Oct 24, 2022

@jeroen As documented in CDRIVER-4484, this is scheduled to be included in the upcoming 1.24.0 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants