Re: suggested compile warnings

["Theodore A. Roth" <troth at openavr dot org>]

On Fri, 23 May 2003, Andrew Cagney wrote:

:) > Hi,
:) >
:) > I've always been configuring gdb with --enable-gdb-build-warnings=-Werror
:) > and thought that there where a bunch of gcc compile warnings issued.
:) > Looking more closely at my builds, that doesn't seem to be true.
:) >
:) > For my own apps, I like to use '-Wall -Werror' as it lets the compiler
:) > catch a lot of my stupid mistakes. I tried '-Wall -Werror' for gdb,
:) > but that seems to be too restrictive for gdb source.
:) >
:) > Does anyone else compile gdb with any of the -W<foo> gcc options?
:) > Is there a recommended list of these which should be used?
:) >
:) > For example, adding -Wunused (without -Werror), turns up 149 wanrings.
:) > Most are unused variables, some are static decl's for functions that
:) > aren't defined. Most if not all of these should be trivial to fix.
:)
:) See: 13.4.3 Compiler Warnings
:) http://sources.redhat.com/gdb/current/onlinedocs/gdbint_13.html#SEC118

Thats what I was looking for. Guess I should have looked in the
index...

I noticed that I was using '-Werror' instead of ',-Werror'. That seems
to quietly disable all the hand picked warnings. D'oh! Now my build is
catching some problems again in my code.

:)
:) Instead of -Wunused, look at the individual -Wunused-function,
:) -Wunused-variable, ...
:)
:) Off hand, other intersting ones are:
:)
:) -Wwrite-strings
:) It's just hard and really messy.   People occasionally chip away at the
:) edges.  Last time I tried, I came across what appeared to be an
:) effective xfree("string"), outch!
:)
:) -W
:) More for the sign VS unsigned checks it throws up, but it throws up some
:) non problems :-(
:)
:) -Wswitch-default
:) -Wswitch-enum
:) -Wswitch-fallthrough (doesn't actually exist)
:) But that could be religious :-)
:)
:) -Wstrict-prototypes
:) -Wmissing-prototypes
:) -Wmissing-declarations
:) But tread carefully, it tends to run a foul of head files.
:)
:) The trick is to first identify warnings that add real value (-Wformat
:) for instance is good++) and then ensure that the fix is not worse than
:) the problem being identified (using a cast to hide a problem, for
:) instance, isn't a good idea vis: xffree ((char *) "a string")).
:)
:) I've appended an old list of the error counts for various flags that was
:) generated by compiling all the cross targets on a Red Hat GNU/Linux 7.2
:) system and then deleting duplicates (which GCC? I don't remember).
:)
:) Have fun.
:) Andrew
:)

Ok, that's a lot of warnings. 8-) Thanks for the info.

Is it worth while cleaning up the simple stuff? I am willing to do
janitorial work since it at least forces me to dig into the code more.

Before I go of half cocked, here's the warnings I get in breakpoint.c:

gcc -c -g -O2    -I. -I../../gdb -I../../gdb/config -DLOCALEDIR="\"/home/roth/local/avr/share/locale\"" -DHAVE_CONFIG_H -I../../gdb/../include/opcode -I../../gdb/../readline/.. -I../bfd -I../../gdb/../bfd  -I../../gdb/../include -I../intl -I../../gdb/../intl  -DMI_OUT=1 -Wall  ../../gdb/breakpoint.c
../../gdb/breakpoint.c: In function `insert_breakpoints':
../../gdb/breakpoint.c:971: warning: value computed is not used
../../gdb/breakpoint.c:717: warning: unused variable `disabled_breaks'
../../gdb/breakpoint.c: In function `can_use_hardware_watchpoint':
../../gdb/breakpoint.c:5526: warning: unused variable `vaddr'
../../gdb/breakpoint.c: In function `do_enable_breakpoint':
../../gdb/breakpoint.c:7207: warning: statement with no effect
../../gdb/breakpoint.c: At top level:
../../gdb/breakpoint.c:5798: warning: `ep_parse_optional_filename' defined but not used

Attached are my fixes. Are these appropriate and obvious fixes? I'm
not excited about the ',' usage in a macro, but it seemed to be used
in a lot of other macros already. [I know the ChangeLog entry is
unacceptable though...]

Ted Roth

Attachment: cleanup-1.diff
Description: Text document