Re: [Plplot-devel] -std-c99 generates lots of warnings and a Java example segfault

[Andrew R. <and...@us...>]

On Fri, Oct 21, 2011 at 09:45:47AM +0100, Andrew Ross wrote:
> 
> I'd independently spotted this doing the same test as you.
> 
> The problem is that the "internal" plplotP.h header is actually used 
> indirectly in the qt examples. In this case HAVE_CONFIG_H is not set to 
> some of the defines (e.g. the check for isfinite) are missing. It is 
> similar to the problem I fixed yesteday with ocamlc not setting 
> HAVE_CONFIG_H. I've moved the required defines to plConfig.h to solve the 
> problem. It would be nice though if plplotP.h was truly internal. In my 
> opinion this should just be used for the build. It is not part of the API 
> and so ideally we shouldn't need to expose the user to it or install it.
> 
> I've done some further testing on another system with a newer version of
> gcc and this has exposed some further warnings. In particular it is 
> now much more picky about const for 2-d arrays, i.e. pointers to pointers
> The problem is that we make use of things like const PLFLT **a. The says 
> that a is a pointer to an array of pointers which in turn point to arrays
> of PLFLT. The const says that the arrays of PLFLT are constant, but NOT
> that the array of pointers is constant. It is therefore trivial to 
> change the pointers in order to change the underlying data. This undermines
> the const guarantee. Strictly one would usually want to do something like
> const PLFLT * const *a which guarantees that both the PLFLT arrays and
> the PLFLT * array are constant. This leads to quite ugly definitions though.
> Invariably const specifiers in C are quite pervasive. Once you have made
> something const it has to propogate through subsequent function calls. 
> 
> Fixing these warnings is likely to result in API changes to the contouring
> / shading functions which use const PLFLT **. The code ugliness and 
> clarity could probably be addressed using some typedefs. E.g.
> something like
> 
> typedef (const PLFLT * const *) PLFLT_array2D_const;
> 
> The real question is, "Is this worth it for a potentially
> pervasive API change just to fix compiler warnings?"
> 
> Another issue I've encountered is that PLplot makes a fair bit of use
> of casting function pointers to PLpointer (i.e. void *) for passing
> between functions. This contravenes ISO C as there is no 
> guarantee that function pointers and object pointers have the same
> size. In practice they do, which is why the code works, but we might
> want to consider a cleaner way of doing this. Again this is a potential
> API change, but it is a clear contravention of standards as opposed to 
> just a compiler warning.

I've managed to avoid all of these in the PLplot code through better
use of casts. There still exist a couple of examples related to 
external libraries (notably calls to lt_dlsym), but there is little
we can do about that.

> I have had no response to my previous email about handling unused parameter
> values. I take that as assent to my proposals, so I will try implementing 
> the UNUSED macro approach, at least for the core library code in src/ .
> Once there is a concrete implementation in place people might be in a
> better position to comment.

I've now implemented this for src/plargs.c as a file with a lot of 
unused parameters. Please look at, test and comment on the implementation.
 
> I realise this is a lot of questions, but given that fixing these
> issues might involve API changes I wanted to seek some sort of
> consensus, or at least give people the chance to comment, before
> making any changes.

Andrew