On Fri, Mar 30, 2018 at 11:16 PM, Sargun Dhillon <sar...@sargun.me> wrote: > On Fri, Mar 30, 2018 at 2:39 PM, Casey Schaufler <ca...@schaufler-ca.com> > wrote: >>> static struct security_hook_list null_hooks[SECURITY_HOOK_COUNT]; >>> -#define HAS_FUNC(SHL, FUNC) (SHL->hook.FUNC) >>> +DEFINE_STATIC_SRCU(security_hook_srcu); >>> + >>> +static inline bool is_null_hook(struct security_hook_list *shl) >>> +{ >>> + union { >>> + void *cb_ptr; >>> + union security_list_options slo; >>> + } hook_options; >>> + >>> + hook_options.slo = shl->hook; >>> + return !hook_options.cb_ptr; >>> +} >> >> I like the HAS_FUNC() approach better. > > Just curious, why? I personally prefer small static inline functions > over macros, if possible.
Generally speaking, small static inline functions are better since they provide type-checking. In this case, though, it looks like you're just doing a cast, but with a union. Why isn't this just: return !!((uintptr_t)shl->hook) ? Though the security_list_options union exists for callback type checking, so really, having HAS_FUNC() with the explicit function you're interested in creates a bit of self-documenting code (even if it always resolves to the above test). -Kees -- Kees Cook Pixel Security