Re: warnings for invalid function casts

Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> writes:
> There are three subplots:

> 1. Changing the return type of load_external_function() and
> lookup_external_function() from PGFunction to a generic pointer type,
> which is what the discussion in [0] started out about.

I feel like what you propose to do here is just shifting the problem
around: we're still casting from a function pointer that describes one
concrete call ABI to a function pointer that describes some other concrete
call ABI. That is, "void (*ptr) (void)" is *not* disclaiming knowledge
of the function's signature, in the way that "void *ptr" disclaims
knowledge of what a data pointer points to. So if current gcc fails to
warn about that, that's just a random and indeed obviously wrong decision
that they might change someday.

Re-reading the original discussion, it seems like what we have to do
if we want to suppress these warnings is to fully buy into POSIX's
assertion that casting between data and function pointers is OK:

Note that conversion from a void * pointer to a function pointer as in:
fptr = (int (*)(int)) dlsym(handle, "my_function");
is not defined by the ISO C standard. This standard requires this
conversion to work correctly on conforming implementations.

I suggest therefore that a logically cleaner solution is to keep the
result type of load_external_function et al as "void *", and have
callers cast that to the required specific function-pointer type,
thus avoiding ever casting between two function-pointer types.
(We could keep most of your patch as-is, but typedef GenericFunctionPtr
as "void *" not a function pointer, with some suitable commentary.)

> 2. There is a bit of cheating in dynahash.c.

It's slightly annoying that this fix introduces an extra layer of
function-call indirection. Maybe that's not worth worrying about,
but I'm tempted to suggest that we could fix it on the same principle
with

hashp->keycopy = (HashCopyFunc) (void *) strlcpy;

> 3. Finally, there is some nonsense necessary in plpython, which is
> annoying but otherwise uninteresting.

Again, it seems pretty random to me that this suppresses any warnings,
but it'd be less so if the intermediate cast were to "void *".

regards, tom lane