On Fri, Oct 14, 2011 at 01:37:30AM -0700, Alan Irwin wrote:
> On 2011-10-14 08:26+0100 Andrew Ross wrote:
>
> > I've committed the first tranche of fixes for compiler warnings.
> >
> > For reference I've been testing with
> > CFLAGS="-O3 -std=c99 -pedantic -Wall -Wextra -Wmissing-prototypes
> > -Wstrict-prototypes -Wnested-externs -Wconversion -Wshadow -Wcast-qual
> > -Wcast-align -Wwrite-strings"
> > CCFLAGS=-O3 -fvisibility=hidden -std=c++98 -pedantic -Wall -Wextra
> > FFLAGS=-O3 -fvisibility=hidden -pedantic -Wall -Wextra
> >
> > When required I have (by hand) added -D_POSIX_C_SOURCE=1 to the flags for
> > specific files (e.g. xwin.c) in order to compile.
> >
> > So far this is just the easy to fix issues (function prototypes, missing
> > braces, missing const for char *, shadowed variables, unused functions.)
> > This is quite intrusive, so please test well.
>
> Ordinary compile flags
>
> CXXFLAGS=-O3 -fvisibility=hidden
> CFLAGS=-O3 -fvisibility=hidden
> FFLAGS=-O3
>
> now give the following warning:
>
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/src/plargs.c: In
> function ?ProcessOpt?:
> /home/software/plplot_svn/HEAD/plplot_cmake_qt/src/plargs.c:1119:
> warning: assignment discards qualifiers from pointer target type
>
> which wasn't there before.
>
> Other than that as of revision 11970 (my style changes on top of
> your compiler warning fixes up to just before that revision),
>
> ctest -j4
>
> shows no obvious issues. By the way, I just discovered that "parallel
> test" option -j4 really does speed ctest execution on a machine with
> two physical processors.
>
> > There will be more to come.
>
> Good work, Andrew!
I have now completed the bulk of this work.
Remaining are
- the issue of unused parameters in function calls (see) below
- some non-standard functions (see below)
- some pointer aliasing rules which we use internally which break strict i
C99 standards
- a number of examples where we neglect the const argument for
const char * , particularly for string literals (i.e. fixed strings given
in quotes).
- swig generated interfaces are very dirty, and much less easily fixed.
- issues with external library headers (e.g. wxwidgets) which break the
C99 / C++98 standards.
The const issue is particularly problematic for Tcl / Tk where the Tcl / Tk
API makes a lot of use of char * when it really means const char *. Casting
does not make these warnings go away as casting cannot make a read-only
string writeable.
Non C99 function calls which we use
strdup - SVr4, 4.3BSD, POSIX.1-2001
finite - SVID 4, BSD. Discouraged - replaced with isfinite.
isfinite - C99, POSIX.1-2001
popen - POSIX.1-2001
pclose - POSIX1-2001
readlink - 4.2BSD, POSIX.1-2001
fdopen - POSIX.1-1990
mkstemp - 4.3BSD, POSIX.1-2001
vfork - 4.3BSD, POSIX.1-2001 but later removed. Discouraged - replaced with
fork.
fork - SVr4, 4.3BSD, POSIX.1-2001
All the additional functions we use are in the POSIX 2001 standard, or earlier
so
compiling with -std=c99 -D_POSIX_C_SOURCE=200112L added to the CFLAGS will work
(i.e. C99 language standard with POSIX 2001 extensions). Some of these are not
supported by some common compilers (e.g. isfinite) so will still need checks.
The issue of unused function parameters is not entirely clear. The warnings are
potentially useful for identifying e.g. errors in parameter names. Removing
unused parameters is often not possible, either because it is an API change
or because the function template requires the arguments (e.g. pltr
or mapform functions which are passed as arguments to PLplot). Ideally unused
parameters would be marked, both to make it clear to the programmer that they
are unused and to silence the compiler warnings. There is no standard way of
doing this. Possibilities include
- Casting the parameter to void, which is a nop but will silence the warning.
This doesn't catch any subsequent use of the parameter. This could be
wrapped in a macro to make it explicit that the purpose is to mark the
parameter as not used.
- Adding a macro around the unused parameter name, e.g.
void func( int UNUSED(x) )
The macro could do various things. GCC has a __attribute__((unused)) which
can be applied to parameters to mark them as unused. One reference I came
across suggested also mangling the parameter name to make sure it isn't
accidentally used anywhere in the function. On non-gcc compilers there
may be other ways of marking the parameter. C++ (but not C) allows you
to comment out the parameter name. Worst case is that the macro does
nothing other than alert the programmer to the fact the parameter is
unused. One example I came across was
#ifdef UNUSED
#elif defined(__GNUC__)
# define UNUSED(x) UNUSED_ ## x __attribute__((unused))
#elif defined(__LCLINT__)
# define UNUSED(x) /*@unused@*/ x
#else
# define UNUSED(x) x
#endif
I have a slight preference for the UNUSED macro approach, but I'm open
to suggestions. Either way we should adopt a consistent PLplot style for
such things if we consider it important.
Silencing the known safe warnings makes any real errors much more
apparent.
Andrew
------------------------------------------------------------------------------
The demand for IT networking professionals continues to grow, and the
demand for specialized networking skills is growing even more rapidly.
Take a complimentary Learning@Ciosco Self-Assessment and learn
about Cisco certifications, training, and career opportunities.
http://p.sf.net/sfu/cisco-dev2dev
_______________________________________________
Plplot-devel mailing list
Plplot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/plplot-devel
