From owner-freebsd-net@FreeBSD.ORG Tue Mar 30 02:22:10 2010 Return-Path: <owner-freebsd-net@FreeBSD.ORG> Delivered-To: net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 44493106566B for <net@freebsd.org>; Tue, 30 Mar 2010 02:22:10 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from fallbackmx06.syd.optusnet.com.au (fallbackmx06.syd.optusnet.com.au [211.29.132.8]) by mx1.freebsd.org (Postfix) with ESMTP id CF0578FC14 for <net@freebsd.org>; Tue, 30 Mar 2010 02:22:09 +0000 (UTC) Received: from mail02.syd.optusnet.com.au (mail02.syd.optusnet.com.au [211.29.132.183]) by fallbackmx06.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id o2U0Gixt013549 for <net@freebsd.org>; Tue, 30 Mar 2010 11:16:44 +1100 Received: from c122-106-158-90.carlnfd1.nsw.optusnet.com.au (c122-106-158-90.carlnfd1.nsw.optusnet.com.au [122.106.158.90]) by mail02.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id o2U0GZ9x030487 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 30 Mar 2010 11:16:37 +1100 Date: Tue, 30 Mar 2010 11:16:35 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> X-X-Sender: bde@delplex.bde.org To: "M. Warner Losh" <imp@bsdimp.com> In-Reply-To: <20100329.115116.385399974524554540.imp@bsdimp.com> Message-ID: <20100330104425.E5195@delplex.bde.org> References: <20100329.115116.385399974524554540.imp@bsdimp.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: net@freebsd.org Subject: Re: Small patch to ipfilter for arm X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD <freebsd-net.freebsd.org> List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-net>, <mailto:freebsd-net-request@freebsd.org?subject=unsubscribe> List-Archive: <http://lists.freebsd.org/pipermail/freebsd-net> List-Post: <mailto:freebsd-net@freebsd.org> List-Help: <mailto:freebsd-net-request@freebsd.org?subject=help> List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-net>, <mailto:freebsd-net-request@freebsd.org?subject=subscribe> X-List-Received-Date: Tue, 30 Mar 2010 02:22:10 -0000 On Mon, 29 Mar 2010, M. Warner Losh wrote: > OK. I'd like to propose the following patch for ipfilter: > > Index: sys/contrib/ipfilter/netinet/ip_compat.h > =================================================================== > --- sys/contrib/ipfilter/netinet/ip_compat.h (revision 205838) > +++ sys/contrib/ipfilter/netinet/ip_compat.h (working copy) > @@ -975,7 +975,6 @@ > # define SPL_NET(x) ; > # define SPL_IMP(x) ; > # define SPL_SCHED(x) ; > -extern int in_cksum __P((struct mbuf *, int)); > # else > # define SPL_SCHED(x) x = splhigh() > # endif /* __FreeBSD_version >= 500043 */ > > This declaration is wrong, and it prevents arm from building ipfilter. Quite likely. It even uses __P(()), whose use is supposed to have gone away ~10 years ago. But this file's purpose is to provide compat cruft like that. Also, the null SPL's in the above have bogus semicolons. > Why is it wrong? Because we have: > > # if (__FreeBSD_version >= 500002) > # include <netinet/in_systm.h> > # include <netinet/ip.h> > # include <machine/in_cksum.h> > # endif > > # if (__FreeBSD_version >= 500043) > ... > <the above code> > ... > # endif > > So, we have in_cksum.h being included *AND* we're defining this > function. However, in_cksum.h is supposed to do this. > > Why don't we see problems today? No architecture except arm has an > assembler in_cksum in the tree. All the other architectures have > > #define in_cksum(a, b) in_cksum_skip(a, b, 0) > > in their headers. Since the above extern uses __P to hide the args, > in_cksum doesn't expand the macro, so we don't see any problems or > conflicts. Not quite. __P(()) does expand args, except for K&R compilers whose use is supposed to have gone away ~20 years ago. However, the macro has precedence over the declaration, so the declaration has no effect (if there is a macro). The ordering of the includes has to be delicate to get the function declared before the macro, else the declaration would be a syntax error. On arm, where we define in_cksum() correctly to return > u_short, there's a conflict. > > So, it would best if we just dropped this one line from ip_compat.h, > since it was always wrong anyway. I agree. This line is only for non-old FreeBSD systems. It can never have had any good effect on these systems, since even if it were correct then it would have forced failure due to -Wredundant-decls (since the delicate include ordering requires including the system header at the correct point and this include is forced in ip_compat.h itself); thus the system definition is always visible and any private declaration gives a redundant-decl). FreeBSD uses -Wredundant-decls to inhibit use of private decls like this. Bruce