On Wed, Aug 20, 2014 at 09:04:41PM +0100, Thomas Adam wrote:
> 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:
> > * 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.)
That doesn't work on many compilers. Of course we can let
configure define it if available and leave it blank if not. I
don't think we'll find any bugs in the core though.
> 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.
Granted, if we add that it'll be somewhat usefull, but don't expect
to find any core bugs with it. The function interfaces are clean.
> > ...
> 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.
Oh yes, it *is* guaranteed for auto variables when you initialise
them:
{
struct foo = {};
}
This cleans out the struct memory. I think is is a good custom to
*not* initialise auto variables ever. I just assign a value right
before I use them (if necessary). That way you get a "may be used
uninitialised" warning which almost always points to a bug.
> Based on what I've written above, we should perhaps augment CFLAGS at
> some point. What do you think?
Well, I've looked over the warnings that the (mvwm) core generates,
and apart from the warning in menus.c which really was a bug (but
had no effect), I'd disable all the other warnings.
At the moment, I'd turn -Wextra on only for the core, removing all
four classes of existing warnings, i.e.
-Wextra -Wno-unused-parameter -Wno-sign-compare -Wno-type-limits
-Wno-missing-field-initializers
Then only three warnings in MvwmPager.c remain, and they point to
real bugs that I'll fix in the separate branch and in fvwm.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
