On Sat, Aug 22, 2015 at 8:07 PM, David Bennett <davidb at pfxcorp.com> wrote:
> Of course that is the aim, as always. > > > > In this particular case, maximally portable code (that will compile and > execute correctly on all conforming compilers) must (a) ensure that the > pointer argument is valid (b) ensure that the length is valid, or zero. > Where reasonably possible both should be done statically. If conditional > code is introduced then it must be tested with all branches covered. > Agreed, and in C89 NULL was a valid pointer argument in this context as long as the length was zero. That has nothing to do with this particular thread, but I referenced it just as a point of "C99 changed the semantics of what is valid, invalidating previously valid code". Projects that took advantage of that can either modify their code to accommodate the newer standard or leave it along claiming it conforms to the intended / desired standard. In that case, it was easy to change to code to be compatible with both standards and it was done. In this case (gcc warning about possible argument order error due to a constant expression) can be equally accommodated, but it's not a matter of standards compliance. It *is* a matter of one of the (or perhaps *the*) most used compilers bellyaching about something that is not wrong or invalid in any way. So is the code modified to suppress the warning, is the warning disabled globally, locally, or just ignored? I can see the benefits to making the change and to not making the change. > As always, it may be the case that one or more compilers may issue > warnings for code that is fully compliant with the standard and fully > tested. Sadly there may even be compilers that compile the code incorrectly > (a compiler bug). The question of how to handle undesired warnings or > compiler bugs on code that is known to be correct and compliant is always a > judgement call. In my opinion the solution chosen should always be as > fine-grained as possible (such as a compiler-specific conditional), but the > downside is that the code can become littered with references to specific > problems in specific compilers and thus become harder to work with. > I agree completely. Most of us don't have to worry about this too much because we target one platform (most code is not written with portability in mind). SQLite is not most projects. > In my opinion changes that are visible to other compilers should be > avoided unless the changed code is an equally valid solution to the problem > at hand and/or the problem affects multiple compilers. In this light adding > an if-test would be an incorrect choice (and would require an additional > set of tests). Suppressing the warning specifically for this compiler would > be a preferable solution. > Agreed for the most part. Just to be clear: I never said "don't make this change" (I don't think). I just wanted to bring up the thought that an if statement *might* degrade performance in a code base that has been carefully tuned to maximize efficiency. I wasn't thinking of test cases or such, just efficiency. That being said, if gcc is generating the warning because a constant expression has a value of zero, an if statement might actually increase performance by providing the compiler with the knowledge that it can completely remove the corresponding memset because the expression will always be false. In that case, add the if might be the exact right thing to do. Again we're to the point of "there is no one universal right solution to this issue". The only thing I can say is: not all warnings are equally bad, and I will review warnings that are generated from third party code (such as SQLite) but I rarely will do anything to try to suppress them. Making my code right is a hard enough task. I don't need to "fix" third party code (as long as it passes testing). -- Scott Robison
