Add parentheses to fix an unsafe macro.

[irwir]

Static analysis flags PNG_FORMAT_NUMBER macro as unsafe. Parentheses ensure correct evaluation in case buffer argument was a complex expression.

[jbowler]

I wrote it that way for good reasons. The only argument permitted is an lvalue array name. There is some chance that if the macro is abused by a future maintainer it will produce a compile error.

[jbowler]

Like I said. To put in another way don't fix things that work.

[irwir]

jbowler, thanks for your remarks. The macro is unsafe. Normally these warnings should be silenced.

Expecting a compilation error in case of misuse is a poor protection. For example, a pointer instead of an array name compiles quietly and might even work in some cases.

[jbowler]

Silencing a warning by changing the semantics of the code is not correct. The macro requires an argument of type (char{]); find me a place where the macro is used with an argument which is not only of that type but also not a local C array of char. Your change is not an appropriate way to silence the warning issued by your compiler. I suggest you take a look at the ways of silencing warnings. libpng already has an extensive list for GCC, but it requires continuous maintenance; the maintainers build libpng with -Wall -Wextra -Werror and then with those specific warnings disabled.

You change is not safe; it might not change the result with current code but that all uses a simple variable name. It would change the semantics of an editing or understanding error.

Now it's not perfect, so if you can provide an analysis which suggests the likely editing errors (i.e. typographical errors made by a maintainer of the code) can be detected using a different approach then that's more interesting.

But the bottom line is still this: a compiler warning is NOT a bug. Compilers, particular GCC generate warnings with every new dawn. They go away by sunset.

@ctruta

Making code changes to established working and substantially unmaintained code just because of a new compiler warning is likely to lead to new bugs not fix old ones. Sure, if someone can show that there really is a bug, perhaps as a result of an analysis caused by a compiler warning, that's useful. But that isn't what is happening here or in several other bug reports that should simply be closed.

libpng 1.6 is substantially working and so far no one has found a significant code bug for a while. Sure lots have been reported and some have been fixed, particularly from fuzzers, but people are resorting to coming up with compiler warnings or convoluted cases involving test programs which are not part of a binary release. These are not bugs.

libpng is stable. Don't destabilize it! Any new code other than hardware support, which is isolated from the main code, should go in libpng1.8; whoever develops that branch is free to mess with things like this, or, for that anything.

[irwir]

Silencing a warning by changing the semantics of the code is not correct.

@jbowler, please care to explain how semantics was easily changed with an additional pair of parentheses.

[irwir]

@jbowler The first two words in the description of this PR are Static analisys, and the title has the word fix. Once again: fix, not silence. This kind of warning cannot be silenced with compilation options - simply because it was a different tool. All this was misread, and the concept of unsafe macro misinterpreted. Please learn the basics, and lower the level of arrogance.

[irwir]

By the way, this tiny issue was promised to be fixed in the #216 - closed only partially completed.