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

krb5: ignore clang warnings on mvsnprintf() #12803

Closed
wants to merge 1 commit into from
Closed

Conversation

bagder
Copy link
Member

@bagder bagder commented Jan 26, 2024

"error: format string is not a string literal"

@vszakats
Copy link
Member

vszakats commented Jan 26, 2024

Proposing this, to have the local printf wrapper verified for format issues:

--- a/lib/krb5.c
+++ b/lib/krb5.c
@@ -434,6 +434,9 @@ static char level_to_char(int level)
 
 /* Send an FTP command defined by |message| and the optional arguments. The
    function returns the ftp_code. If an error occurs, -1 is returned. */
+static int ftp_send_command(struct Curl_easy *data, const char *message, ...)
+  CURL_PRINTF(2, 3);
+
 static int ftp_send_command(struct Curl_easy *data, const char *message, ...)
 {
   int ftp_code;

Follow-up to 0923012

@bagder
Copy link
Member Author

bagder commented Jan 26, 2024

Much better!

"error: format string is not a string literal"

Follow-up to 0923012 which made the warning appear

Assisted-by: Viktor Szakats
@bagder bagder closed this in aee4ebe Jan 26, 2024
@bagder bagder deleted the bagder/krb-clang branch January 26, 2024 22:28
@vszakats
Copy link
Member

Oops, I meant the CURL_PRINTF() patch as an extra over the initial patch. The warning suppression is still necessary. Sorry for being unclear.

@bagder
Copy link
Member Author

bagder commented Jan 27, 2024

I don't get any warnings with clang anymore after this!

@vszakats
Copy link
Member

That's interesting. It seems the compiler is clever enough to apply this knowledge to the function itself. It also means that some of these warning suppressions added earlier (called out "looking shit" in a comment), may be redundant after all.

@vszakats
Copy link
Member

Following-up on this: #12812

Turns out most of them became redundant after adding CURL_PRINTF().
Could also eliminate two more with minor code changes.

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

2 participants