This is the mail archive of the gdb-patches@sourceware.cygnus.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: Clean up compiler warnings


Jim Kingdon wrote:
> 
> I don't know who is supposed to approve such things these days, but
> the enclosed patch cleans up some of the compiler warnings (there are
> many left :-().

Presumably me (perhaps JimB (?) who last touched some of that code.)

So you know and don't start doing unnecessary work, the suggested list
of warning flags for 5.0 is no more than:

enable_build_warnings=`echo -Werror\
,-Wimplicit\
,-Wreturn-type\
,-Wcomment\
,-Wtrigraphs\
,-Wformat\
,-Wparentheses\
,-Wpointer-arith\
,-Woverloaded-virtual\

As you note, there are many more warnings if all the flags are used. 
This set has proved to be a good compromise between detecting common
silly mistakes (such as printf and no-return functions) and changing
code just for the sake of flushing warnings.

Post 5.0, I expect the -Werror bar to firstly be formalized and then
raised to a new level.

As for the actual changes, they all look pretty good - especially the
bit where you delete ``extern''s from .c files :-)

One strategic change I would suggest though is with switches:

> --- ch-exp.c    1999/08/09 21:33:19     1.1.1.3
> +++ ch-exp.c    2000/02/07 19:42:56
> @@ -2192,8 +2192,8 @@
>             case LOC_OPTIMIZED_OUT:
>               error ("Symbol \"%s\" names no location.", inputname);
>               break;
> -           case LOC_UNRESOLVED:
> -             error ("unhandled SYMBOL_CLASS in ch_lex()");
> +           default:
> +             internal_error ("unhandled SYMBOL_CLASS in ch_lex()");
>               break;
>             }
>         }

When switching on an enum (as in this case) I've typically found it
better to explicitly list all cases rather than use a default.  That
way, when a new element is added to the enum, the -Wswitch warning is
available as a tool to the programmer to identify where code needs to be
altered.  As an aside, that is why a switch on an enum rather than an
if/elsif chain often proves more resilient to long term maintenance.

	enjoy,
		Andrew

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]