On Fri, Feb 06, 2004 at 10:06:14AM +0000, Ben Laurie wrote:
> Joe Orton wrote:
> >From delving through C99, this appears to be a valid warning, since the
> >code has undefined behaviour by section 6.3.2.3 para 8. (which says, to
> >try and paraphrase: behaviour is undefined if you call a function
> >through a function pointer where the function and the function pointer
> >don't have compatible types)
>
> Sigh. OK, how do I get hold of C99?
Only by buying a copy from ISO I think.
> Surely the point is that the function and the function pointer actually
> _do_ have compatible types, but we hold the pointer in a variable that
> doesn't. That is, we cast it to an incompatible type for storage, then
> cast it back.
The issue is the call to apr_dynamic_fn_register, here's how I read it:
(((void (*)(const char *, APR_OPTIONAL_FN_TYPE(name) *))
&apr_dynamic_fn_register)(#name,name))
this takes a function pointer (&apr_dynamic_fn_register), then casts it
to a different type (void (*)(const blah blah)), then calls the function
using this type. That has undefined behaviour, since the type of
apr_dynamic_fn_register is not compatible with the (void (*)(const blah
blah) type.
Here's a simpler version of the patch: httpd-2.0 builds with "gcc
version 3.4.0 20040121" at -Wall -Werror and handles requests fine with
this applied. Deliberately introducing a type-unsafe use of either
macro gets a warning like:
mpm_common.c: In function `ap_mpm_rewrite_args':
mpm_common.c:884: warning: initialization from incompatible pointer type
--- apr-util-0.9.4/include/apr_optional.h.gcc34
+++ apr-util-0.9.4/include/apr_optional.h
@@ -109,9 +109,16 @@
* confusingly but correctly, the function itself can be static!
* @param name The name of the function
*/
+#ifdef __GNUC__
+#define APR_REGISTER_OPTIONAL_FN(name) do { \
+ APR_OPTIONAL_FN_TYPE(name) *apu__opt = name; \
+ apr_dynamic_fn_register(#name,(apr_opt_fn_t *)apu__opt); \
+} while (0)
+#else
#define APR_REGISTER_OPTIONAL_FN(name) \
(((void (*)(const char *, APR_OPTIONAL_FN_TYPE(name) *)) \
&apr_dynamic_fn_register)(#name,name))
+#endif
/** @internal
* Private function! DO NOT USE!
--- apr-util-0.9.4/include/apr_optional_hooks.h.gcc34
+++ apr-util-0.9.4/include/apr_optional_hooks.h
@@ -99,10 +99,17 @@
* @param nOrder an integer determining order before honouring aszPre and
aszSucc (for example HOOK_MIDDLE)
*/
+#ifdef __GNUC__
+#define APR_OPTIONAL_HOOK(ns,name,pfn,aszPre,aszSucc,nOrder) do { \
+ ns##_HOOK_##name##_t *apu__hook = pfn; \
+ apr_optional_hook_add(#name,(apr_opt_fn_t *)apu__hook,aszPre, aszSucc,
nOrder); \
+} while (0)
+#else
#define APR_OPTIONAL_HOOK(ns,name,pfn,aszPre,aszSucc,nOrder) \
((void (APR_THREAD_FUNC *)(const char *,ns##_HOOK_##name##_t *,const char
* const *, \
const char * const
*,int))&apr_optional_hook_add)(#name,pfn,aszPre, \
aszSucc, nOrder)
+#endif
/**
* @internal
