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: more -Wformat fixes #12540

Closed
wants to merge 1 commit into from
Closed

build: more -Wformat fixes #12540

wants to merge 1 commit into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Dec 16, 2023

  • memdebug: update to not trigger -Wformat-nonliteral warnings.
  • imap: mark imap_sendf() with CURL_PRINTF().
  • tool_msgs: mark static function with CURL_PRINTF().

Follow-up to 3829759 #12489

Closes #12540

@vszakats
Copy link
Member Author

vszakats commented Dec 16, 2023

Existing memdebug code sometimes used %ld for the socket type, but the CURL_FORMAT_SOCKET_T macro doesn't, so this might need more tweaking.

vszakats referenced this pull request Dec 16, 2023
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
- add `CURL_PRINTF()` to `imap_sendf()`.

- update memdebug functions to no longer trigger `-Wformat-nonliteral`
  warnings.

Follow-up to 3829759 curl#12489

Closes #xxxxx
@vszakats vszakats closed this in 0923012 Dec 18, 2023
@vszakats vszakats deleted the printfmore branch December 18, 2023 14:58
vszakats added a commit that referenced this pull request 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

Reviewed-by: Daniel Stenberg
Closes #12812
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

1 participant