On Wed, Aug 20, 2014 at 08:20:34PM +0100, Dominik Vogt wrote:
> On Tue, Aug 19, 2014 at 10:49:53AM +0100, Thomas Adam wrote:
> > I've added a note to the TODO about fixing up compiler warning with -Wextra
> > as well, since there's a tonne of warnings regarding signed vs. unsigned and
> > unused variables, etc. At some point we ought to look at that.
>
> I already tried that at some time, but they are almost all just
> false positives:
I don't mind appraising those. What I was meaning by "fix them" as
either make code changes, or explicitly set -Wno-foo-bar options, and
thus still keep -Wextra to catch the other cases.
> * Unused parameters are very common in fvwm by design (command
> parameters, arguments of library calls) and there's no sensible,
> compiler-independent way to get rid of the warnings.
So in other projects I often do:
#define unused __attribute__ ((unused))
I can't speak for other compilers, but that'll certainly work for GCC
and related compilers (even those found on Solaris as well, in my
experience.)
Where it's useful, is to acknowledge the fact that the variable is meant
to be unused---such as the API used for callbacks, but the callback
might not do anything useful yet, etc. Marking variable as unused shows
the intent, rather than "I forgot", for instance.
> * Comparisons between signed and unsigned are hardly ever a
> problem, especially not in fvwm which does hardly ever use the
> full range of an int or even a short. These things have been
> cleaned up during the 64-bit coding anyway.
Sure, I can understand that: -Wno-sign-compare
> * Missing initializers are somewhat interesting, but in my eyes
> nothing that warrents a warning. It's well defined in C how
> fields that are not mentioned in the initializer are filled,
> and I see no reason to forbid that. We're not talking about
> c++.
Well, definitions declared as 'static' will have their default value set
appropriately. That's not guaranteed for non-static auto variables.
Again, I'm happy to use: -Wno-uninitialized
> The modules could use some cleanup, though. I'll make a branch
> dv/warning-fixes and push what I fix there.
Thank you.
Based on what I've written above, we should perhaps augment CFLAGS at
some point. What do you think?
> What I'd really like to see is a git commit hook that rejects
> commits that introduce certain whitespace errors in C code source
> files:
I have something for this. I'll dig it up (it's at work) and I'll band
it about here for a bit to get feedback. How's that? I definitely
think it's a good idea and would help enforce the bare-minimum
consistency checks.
-- Thomas Adam
--
"Deep in my heart I wish I was wrong. But deep in my heart I know I am
not." -- Morrissey ("Girl Least Likely To" -- off of Viva Hate.)
