Subject: kern/32645: wrong expression in VLAN_ATTACHED macro (if_ether.h)
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Pavel Cahyna <pcah8322@artax.karlin.mff.cuni.cz>
List: netbsd-bugs
Date: 01/26/2006 23:25:00
>Number: 32645
>Category: kern
>Synopsis: wrong expression in VLAN_ATTACHED macro (if_ether.h)
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: kern-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Thu Jan 26 23:25:00 +0000 2006
>Originator: Pavel Cahyna
>Release: NetBSD 3.0_RC5
>Organization:
>Environment:
System: NetBSD beta 3.0_RC5 NetBSD 3.0_RC5 (EV56) #3: Mon Dec 12 20:28:20 CET 2005 pavel@beta:/usr/src/sys/arch/alpha/compile/EV56 alpha
Architecture: alpha
Machine: alpha
>Description:
if_ether.h has
/* test if any VLAN is configured for this interface */
#define VLAN_ATTACHED(ec) (&(ec)->ec_nvlans > 0)
-> has higher priority than &, so this code takes the address of
ec->ec_nvlans and compares it against a NULL pointer! I doubt that this is
the itended effect...
The comparison will be always true, so VLAN_OUTPUT_TAG will always try
to find the tag. This probably won't have any other negative effect
than performance degradation in a case without any VLAN.
>How-To-Repeat:
code analysis. Change the 0 to 1, which would be still syntactically
valid for an integer expression and see:
$ make COPTS+=-fsyntax-only if_gsip.o
# compile BETA/if_gsip.o
cc -mno-fp-regs -ffreestanding -g -fsyntax-only -Werror -Wall -Wno-main -Wno-format-zero-length -Wpointer-arith -Wmissing-prototypes -Wstrict-prototypes -Wno-sign-compare -fno-zero-initialized-in-bss -Dalpha -I. -I../../../../arch -I../../../.. -nostdinc -DDIAGNOSTIC -DMSGBUFSIZE=1048576 -DUFS_DIRHASH -DLKM -DAUDIO_DEBUG=5 -DMAXUSERS=32 -D_KERNEL -D_KERNEL_OPT -I../../../../dist/ipf -c ../../../../dev/pci/if_gsip.c
In file included from ../../../../dev/pci/if_gsip.c:43:
../../../../dev/pci/if_sip.c: In function `gsip_start':
../../../../dev/pci/if_sip.c:1366: warning: comparison between pointer and integer
../../../../dev/pci/if_sip.c: In function `gsip_init':
../../../../dev/pci/if_sip.c:2486: warning: comparison between pointer and integer
../../../../dev/pci/if_sip.c:2499: warning: comparison between pointer and integer
../../../../dev/pci/if_sip.c:2508: warning: comparison between pointer and integer
*** Error code 1
>Fix:
--- if_ether.h.~1.38.~ Sun Feb 20 16:41:48 2005
+++ if_ether.h Fri Jan 27 00:14:38 2006
@@ -273,7 +273,7 @@
((*(u_int *)(mtag + 1)) & 4095)
/* test if any VLAN is configured for this interface */
-#define VLAN_ATTACHED(ec) (&(ec)->ec_nvlans > 0)
+#define VLAN_ATTACHED(ec) ((ec)->ec_nvlans > 0)
void ether_ifattach(struct ifnet *, const u_int8_t *);
void ether_ifdetach(struct ifnet *);