apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ben Laurie <...@algroup.co.uk>
Subject Re: [PATCH] optional hook macros
Date Fri, 06 Feb 2004 10:06:14 GMT
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?

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. So, the warning, though correct in theory, is wrong in 
practice. Can we fix it by casting the storage instead?

> Given that, I don't see how these macros can be written to be both
> conformant C and type-safe.  It can be be made type-safe using GCC
> extensions, for instance, as below: any better ideas?
> 
> It might be prudent to also make the !__GNUC__ case valid C but not
> type-safe, by casting the pfn argument rather than casting
> apr_dynamic_fn_register itself.
> Index: include/apr_optional.h
> ===================================================================
> RCS file: /home/cvs/apr-util/include/apr_optional.h,v
> retrieving revision 1.12
> diff -u -r1.12 apr_optional.h
> --- include/apr_optional.h	16 Nov 2003 01:50:10 -0000	1.12
> +++ include/apr_optional.h	5 Feb 2004 16:36:19 -0000
> @@ -105,9 +105,18 @@
>   * 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__opt1; \
> +  typeof(name) apu__opt2; \
> +  (void) (&apu__opt1 == &apu__opt2); \
> +  apr_dynamic_fn_register(#name,(apr_opt_fn_t *)name); \
> +} 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! 
> Index: include/apr_optional_hooks.h
> ===================================================================
> RCS file: /home/cvs/apr-util/include/apr_optional_hooks.h,v
> retrieving revision 1.9
> diff -u -r1.9 apr_optional_hooks.h
> --- include/apr_optional_hooks.h	1 Jan 2003 00:02:20 -0000	1.9
> +++ include/apr_optional_hooks.h	5 Feb 2004 16:36:19 -0000
> @@ -99,10 +99,20 @@
>   * @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__opt1; \
> +  typeof(pfn) apu__opt2; \
> +  (void) (&apu__opt1 == &apu__opt2); \
> +  apr_optional_hook_add(#name,(apr_opt_fn_t *)pfn,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
> 
> 


-- 
http://www.apache-ssl.org/ben.html       http://www.thebunker.net/

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff

Mime
View raw message