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

Fix format-security GCC error (GH-64) #65

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@akien-mga
Copy link

akien-mga commented Feb 8, 2018

Like Fedora, Mageia builds all its packages with -Werror=format-security, so even if this code branch is actually never used as mentioned in #64 (comment), at least it builds :)

@lluchs

This comment has been minimized.

Copy link
Member

lluchs commented Feb 8, 2018

Although your commit fixes the warning, it's incorrect and breaks the code using the function.

There are two cases here:

  1. The function is called as Error([...], "this is msg");. The template expands to:
std::string message = 0 > 0 ? strprintf("this is msg") : "this is msg";

You'll see that the middle part is dead code (0 is never greater than 0). GCC still issues the -Wformat-security warning incorrectly.

  1. The function is called as Error([...], "this is msg with %d formatting", 42);. The template expands to:
std::string message = 1 > 0 ? strprintf("this is msg with %d formatting", 42) : "this is msg with %d formatting";

Now strprintf is always called and doesn't cause any warnings. It will print "this is msg with 42 formatting". With your change, it will not do formatting in msg and will instead print "this is msg with %d formatting".

@lluchs lluchs closed this Feb 8, 2018

@lluchs

This comment has been minimized.

Copy link
Member

lluchs commented Feb 8, 2018

If you really don't want to disable -Werror=format-security globally, we may merge a patch that uses #pragmas to disable the warnings for that function only, if you want to try doing that :)

Edit: Actually, there's an easier solution: we can just add overloads to Warning()/Error() without the extra formatting arguments that don't call strprintf(). That would duplicate some code, though.

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