svn commit: r201063 - user/luigi/ipfw3-head/sys/netinet/ipfw

[Bruce Evans]

On Sun, 27 Dec 2009, Luigi Rizzo wrote:

> Log:
>  use a less obfuscated construct to call the hook/unhook functions
>
> Modified:
>  user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_pfil.c

Better unobfuscation:

> Modified: user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_pfil.c
> ==============================================================================
> --- user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_pfil.c	Sun Dec 27 21:58:48 2009	(r201062)
> +++ user/luigi/ipfw3-head/sys/netinet/ipfw/ip_fw_pfil.c	Sun Dec 27 22:13:19 2009	(r201063)
> @@ -329,18 +329,17 @@ ipfw_divert(struct mbuf **m0, int incomi
> static int
> ipfw_hook(int onoff, int pf)
> {
> +	const int arg = PFIL_IN | PFIL_OUT | PFIL_WAITOK;

Don't add this obfuscation (a constant used only once stored in a variable
used only once, just to avoid 2 long lines (1 after my change).

> 	struct pfil_head *pfh;
> -	int (*fn)(int (*pfil_func)(void *, struct mbuf **,
> -		    struct ifnet *, int, struct inpcb *),
> -		   void *, int, struct pfil_head *);
> -

OK.  There is an even more negative need for a variable to hold this
constant that is only used once, since declaring the variable is messy
and initializing it at best wastes space.

>
> 	pfh = pfil_head_get(PFIL_TYPE_AF, pf);
> 	if (pfh == NULL)
> 		return ENOENT;
>
> -	fn = (onoff) ? pfil_add_hook : pfil_remove_hook;
> -	(void)fn(ipfw_check_hook, NULL, PFIL_IN | PFIL_OUT | PFIL_WAITOK, pfh);
> +	if (onoff)
> +		(void)pfil_add_hook(ipfw_check_hook, NULL, arg, pfh);
> +	else
> +		(void)pfil_remove_hook(ipfw_check_hook, NULL, arg, pfh);

Instead, change this to:

 	(void)(onoff ? pfil_add_hook : pfil_remove_hook)(ipfw_check_hook, NULL,
 	    PFIL_IN | PFIL_OUT | PFIL_WAITOK, pfh);

The cast to (void) may be a style bug.  Is success guaranteed?

Bruce