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

build: delete/replace clang warning pragmas #12812

Closed
wants to merge 3 commits into from
Closed

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Jan 27, 2024

  • delete redundant warning suppressions for -Wformat-nonliteral.
    This now relies on CURL_PRINTF() and it's theoratically possible
    that this macro isn't active but the warning is. We're ignoring this
    as a corner-case here.

  • replace two pragmas with code changes to avoid the warnings.

Follow-up to aee4ebe #12803
Follow-up to 0923012 #12540
Follow-up to 3829759 #12489

Closes #12812

@vszakats vszakats changed the title try dropping clang diagnostic ignored "-Wformat-nonliteral" build: delete/replace clang pragmas Jan 27, 2024
@vszakats vszakats added the build label Jan 27, 2024
@vszakats vszakats marked this pull request as ready for review January 27, 2024 20:00
@vszakats vszakats changed the title build: delete/replace clang pragmas build: delete/replace clang warning pragmas Jan 27, 2024
vszakats referenced this pull request Jan 27, 2024
https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html
as of 2023-11-29 [1].

Enable new recommended warnings (except `-Wsign-conversion`):

- enable `-Wformat=2` for clang (in both cmake and autotools).
- add `CURL_PRINTF()` internal attribute and mark functions accepting
  printf arguments with it. This is a copy of existing
  `CURL_TEMP_PRINTF()` but using `__printf__` to make it compatible
  with redefinting the `printf` symbol:
  https://gcc.gnu.org/onlinedocs/gcc-3.0.4/gcc_5.html#SEC94
- fix `CURL_PRINTF()` and existing `CURL_TEMP_PRINTF()` for
  mingw-w64 and enable it on this platform.
- enable `-Wimplicit-fallthrough`.
- enable `-Wtrampolines`.
- add `-Wsign-conversion` commented with a FIXME.
- cmake: enable `-pedantic-errors` the way we do it with autotools.
  Follow-up to d5c0351 #2747
- lib/curl_trc.h: use `CURL_FORMAT()`, this also fixes it to enable format
  checks. Previously it was always disabled due to the internal `printf`
  macro.

Fix them:

- fix bug where an `set_ipv6_v6only()` call was missed in builds with
  `--disable-verbose` / `CURL_DISABLE_VERBOSE_STRINGS=ON`.
- add internal `FALLTHROUGH()` macro.
- replace obsolete fall-through comments with `FALLTHROUGH()`.
- fix fallthrough markups: Delete redundant ones (showing up as
  warnings in most cases). Add missing ones. Fix indentation.
- silence `-Wformat-nonliteral` warnings with llvm/clang.
- fix one `-Wformat-nonliteral` warning.
- fix new `-Wformat` and `-Wformat-security` warnings.
- fix `CURL_FORMAT_SOCKET_T` value for mingw-w64. Also move its
  definition to `lib/curl_setup.h` allowing use in `tests/server`.
- lib: fix two wrongly passed string arguments in log outputs.
  Co-authored-by: Jay Satiro
- fix new `-Wformat` warnings on mingw-w64.

[1] https://github.com/ossf/wg-best-practices-os-developers/blob/56c0fde3895bfc55c8a973ef49a2572c507b2ae1/docs/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C%2B%2B.md

Closes #12489
Copy link
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

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

Nice!

@vszakats vszakats closed this in 5b286c2 Jan 27, 2024
@vszakats vszakats deleted the test1 branch January 27, 2024 21:20
vszakats added a commit that referenced this pull request Jan 28, 2024
- tool_msgs: delete redundant `-Wformat-nonliteral` suppression pragma.

- whitespace formatting in `mprintf.h`, lib518, lib537.

- lib518: fix wrong variable in `sizeof()`.

- lib518: bump variables to `rlim_t`.
  Follow-up to e2b3941 #1469

- lib518: sync error message with lib537
  Follow-up to 365322b

- lib518, lib537: replace `-Wformat-nonliteral` suppression pragmas
  by reworking test code.

Follow-up to 5b286c2 #12812
Follow-up to aee4ebe #12803
Follow-up to 0923012 #12540
Follow-up to 3829759 #12489

Reviewed-by: Daniel Stenberg
Closes #12814
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants