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

mprintf: Ignore non-literal format string #8740

Closed
wants to merge 1 commit into from

Conversation

gjasny
Copy link
Contributor

@gjasny gjasny commented Apr 24, 2022

Using the non-literal format string here is intentional and the warning leads to noise in the best case or an error in the worst.

@bagder
Copy link
Member

bagder commented Apr 24, 2022

How do you to get this warning?

@bagder bagder added the tidy-up label Apr 24, 2022
@gjasny
Copy link
Contributor Author

gjasny commented Apr 25, 2022

We have a custom CMake toolchain that sets -Wformat=2 in the CMAKE_C_FLAGS.

@danielgustafsson
Copy link
Member

What if someone has a toolchain or process which relies on trapping that?

@gjasny
Copy link
Contributor Author

gjasny commented Apr 25, 2022

AFAIU the compiler warning was added to not pass a user-controlled format string to printf like in this example:

void log(const char* msg) {
  printf(msg); // use printf("%s", msg) instead
}

As heuristic for non-user controlled strings the compiler looks for a string literal (which is always compile-time defined). But here in dprintf_formatf the format string is assembled on-the-fly but still not user-controlled. This renders the compiler warning into a false-positive because the format string is not user-controlled.

To suppress the warning we save all the enabled warnings, disable the format-nonliteral warning and after the function call restore all the warnings.

I cannot think of a case where one would like to see the warning anyways.

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.

Seems small and innocuous enough as long as we don't have to do this dance on many places.

@bagder bagder closed this in 5367899 May 16, 2022
@bagder
Copy link
Member

bagder commented May 16, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants