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
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"
