pcnet32 compilation fix for 2.4.3pre6

Hi,
It looks like a not fully merged patch from Alan's tree:

drivers/net/net.o: In function `pcnet32_open':
drivers/net/net.o(.text+0x3bb9): undefined reference to is_valid_ether_addr' drivers/net/net.o: In function pcnet32_probe1':
drivers/net/net.o(.text.init+0x5fa): undefined reference to `is_valid_ether_addr'

hmm, on second thought, I think I would prefer the attached patch (compiled but not tested).

Hardware usually returns all 1's when it's not present, or not working, so think checking for addresses filled with 1's is a good idea too.

hmm, on second thought, I think I would prefer the attached patch (compiled but not tested).

Hardware usually returns all 1's when it's not present, or not working, so think checking for addresses filled with 1's is a good idea too.

I also took the patch from alan's tree and made the memcmp against six-byte 'zaddr' rather than a seven-byte string :)

Index: include/linux/etherdevice.h
===================================================================
RCS file: /cvsroot/gkernel/linux_2_4/include/linux/etherdevice.h,v retrieving revision 1.1.1.14.4.2
diff -u -r1.1.1.14.4.2 etherdevice.h

linux/etherdevice.h 2001/03/22 13:37:15 renamed from linux/etherdevice.h 2001/03/21 14:10:50 1.1.1.14.4.2 +19 -0

@@ -46,6 +46,25 @@
        memcpy (dest->data, src, len);
 }
 
+/**
+ * is_valid_ether_addr - Determine if the given Ethernet address is valid
+ * @addr: Pointer to a six-byte array containing the Ethernet address
+ *
+ * Check that the Ethernet address (MAC) is not 00:00:00:00:00:00, is not
+ * a multicast address, and is not FF:FF:FF:FF:FF:FF.
+ *
+ * Return true if the address is valid.
+ */
+static inline int is_valid_ether_addr( u8 *addr )
+{
+       const char zaddr[6] = {0,};
+       const char faddr[6] = {0xFF,0xFF,0xFF,0xFF,0xFF,0xFF};
+
+       return !(addr[0]&1) &&
+              memcmp( addr, zaddr, 6) &&
+              memcmp( addr, faddr, 6);
+}
+
 #endif
 
 #endif /* _LINUX_ETHERDEVICE_H */

The "!(addr[0]&1)" part of the test already catches the ff's case, so that is redundant.
Using 6 bytes instead of 7 is an improvement.

Quoted 3 lines · Eli Carter

The "!(addr[0]&1)" part of the test already catches the ff's case, so that is redundant.
Using 6 bytes instead of 7 is an improvement.

oops. Thanks, updated patch attached. My patch also adds inline source docs, and uses 'static inline' instead of 'static __inline__', two small style improvements.

------------------------------------------------------------------------
Index: include/linux/etherdevice.h
===================================================================
RCS file: /cvsroot/gkernel/linux_2_4/include/linux/etherdevice.h,v retrieving revision 1.1.1.14.4.2
diff -u -r1.1.1.14.4.2 etherdevice.h

linux/etherdevice.h 2001/03/22 14:44:51 renamed from linux/etherdevice.h 2001/03/21 14:10:50 1.1.1.14.4.2 +9 -0

@@ -46,6 +46,22 @@
        memcpy (dest->data, src, len);
 }
 
+/**
+ * is_valid_ether_addr - Determine if the given Ethernet address is valid
+ * @addr: Pointer to a six-byte array containing the Ethernet address
+ *
+ * Check that the Ethernet address (MAC) is not 00:00:00:00:00:00, is not
+ * a multicast address, and is not FF:FF:FF:FF:FF:FF.
+ *
+ * Return true if the address is valid.
+ */

Hmm... I used __inline__ because the other function in the same headerfile used it... What is the difference between the two, and is one depricated now? (And what about in 2.2.x?)

with the new ansi standard, this use of __inline__ is no longer necessary,

Quoted 2 lines · [email protected] writes:

with the new ansi standard, this use of __inline__ is no longer necessary,

This is not correct. Since the semantics of inline in C99 and gcc differ all code which depends on the gcc semantics should continue to use __inline__ since this keyword will hopefully forever signal the gcc semantics.

Quoted 2 lines · [email protected] writes:

with the new ansi standard, this use of __inline__ is no longer necessary,

This is not correct. Since the semantics of inline in C99 and gcc differ all code which depends on the gcc semantics should continue to use __inline__ since this keyword will hopefully forever signal the gcc semantics.

Quoted 6 lines · Ulrich Drepper

Quoted 2 lines · [email protected] writes:

with the new ansi standard, this use of __inline__ is no longer necessary,

Quoted 4 lines

This is not correct. Since the semantics of inline in C99 and gcc differ all code which depends on the gcc semantics should continue to use __inline__ since this keyword will hopefully forever signal the gcc semantics.

Unfortunately, it seems that gcc will define __inline__ as a synonym for inline, whatever inline is currently in use. I asked this on the gcc list a while ago. The archive there should have the replies.

Mmmm.... documentation. Yummy. ;)

When I submitted the original patch, someone wanted to add the ff's check as well... to reduce the number of people who make that suggestion, perhaps the comment should read:

+ * Check that the Ethernet address (MAC) is not a multicast address or
+ * FF:FF:FF:FF:FF:FF (by checking addr[0]&1) and is not 00:00:00:00:00:00.
+ *

Does that make it clear that both cases are covered by the one test?

Hmm... I used __inline__ because the other function in the same headerfile used it... What is the difference between the two, and is one depricated now? (And what about in 2.2.x?)