From owner-svn-src-user@FreeBSD.ORG Mon Dec 28 17:35:08 2009 Return-Path: Delivered-To: svn-src-user@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8F7CD106566B for ; Mon, 28 Dec 2009 17:35:08 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from fallbackmx10.syd.optusnet.com.au (fallbackmx10.syd.optusnet.com.au [211.29.132.251]) by mx1.freebsd.org (Postfix) with ESMTP id 260C18FC0C for ; Mon, 28 Dec 2009 17:35:07 +0000 (UTC) Received: from mail03.syd.optusnet.com.au (mail03.syd.optusnet.com.au [211.29.132.184]) by fallbackmx10.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id nBSFcJqt024561 for ; Tue, 29 Dec 2009 02:38:19 +1100 Received: from c220-239-235-55.carlnfd3.nsw.optusnet.com.au (c220-239-235-55.carlnfd3.nsw.optusnet.com.au [220.239.235.55]) by mail03.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id nBSFcFYl020603 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 29 Dec 2009 02:38:16 +1100 Date: Tue, 29 Dec 2009 02:38:15 +1100 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Luigi Rizzo In-Reply-To: <200912272213.nBRMDJAC069043@svn.freebsd.org> Message-ID: <20091229021846.U46429@delplex.bde.org> References: <200912272213.nBRMDJAC069043@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: src-committers@FreeBSD.org, svn-src-user@FreeBSD.org Subject: Re: svn commit: r201063 - user/luigi/ipfw3-head/sys/netinet/ipfw X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Dec 2009 17:35:08 -0000 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