Date: Tue, 29 Dec 2009 02:38:15 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Luigi Rizzo <luigi@FreeBSD.org> Cc: src-committers@FreeBSD.org, svn-src-user@FreeBSD.org Subject: Re: svn commit: r201063 - user/luigi/ipfw3-head/sys/netinet/ipfw Message-ID: <20091229021846.U46429@delplex.bde.org> In-Reply-To: <200912272213.nBRMDJAC069043@svn.freebsd.org> References: <200912272213.nBRMDJAC069043@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20091229021846.U46429>