apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Orton <jor...@redhat.com>
Subject Re: [PATCH] optional hook macros
Date Fri, 06 Feb 2004 10:31:07 GMT
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

Mime
View raw message