[U-Boot] [PATCH 10/28] net: cosmetic: Improve variable names and code readability

Joe Hershberger joe.hershberger at ni.com
Fri Jan 20 01:53:13 CET 2012


Rename parameter len to payload_len in NetSendUDPPacket: this name
   more explicitly claims that it does not include the header size
Rename CDPHandler to CDPReceive: this is not called as a handler,
   so don't name it that way
Rename OPT_SIZE to OPT_FIELD_SIZE: clearer constant name and also
   remove related BOOTP_SIZE which was unused and doesn't take into
   account VLAN packets
Rename tmp to reply_ip_addr in arp.c
Alphabetize includes in net.c
Replace magic numbers in arp.c with constants
Add a more explicit comment about 802.2

Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
Cc: Joe Hershberger <joe.hershberger at gmail.com>
Cc: Wolfgang Denk <wd at denx.de>
---
 include/net.h |   11 +++++++-
 net/arp.c     |   38 ++++++++++++++++----------------
 net/bootp.c   |   10 ++++----
 net/bootp.h   |    7 ++---
 net/cdp.c     |    2 +-
 net/cdp.h     |    2 +-
 net/net.c     |   67 +++++++++++++++++++++++++++++---------------------------
 7 files changed, 73 insertions(+), 64 deletions(-)

diff --git a/include/net.h b/include/net.h
index 9de1181..add2080 100644
--- a/include/net.h
+++ b/include/net.h
@@ -169,7 +169,8 @@ struct Ethernet_t {
 };
 
 #define ETHER_HDR_SIZE	14		/* Ethernet header size		*/
-#define E802_HDR_SIZE	22		/* 802 ethernet header size	*/
+				/* 802.2 + SNAP + ethernet header size	*/
+#define E802_HDR_SIZE	(ETHER_HDR_SIZE + 8)
 
 /*
  *	Ethernet header
@@ -231,7 +232,9 @@ struct ARP_t {
 #   define ARP_ETHER	    1		/* Ethernet  hardware address	*/
 	ushort		ar_pro;		/* Format of protocol address	*/
 	uchar		ar_hln;		/* Length of hardware address	*/
+#   define ARP_HLEN	6
 	uchar		ar_pln;		/* Length of protocol address	*/
+#   define ARP_PLEN	4
 	ushort		ar_op;		/* Operation			*/
 #   define ARPOP_REQUEST    1		/* Request  to resolve  address	*/
 #   define ARPOP_REPLY	    2		/* Response to previous request	*/
@@ -245,6 +248,10 @@ struct ARP_t {
 	 * specific hardware/protocol combinations.
 	 */
 	uchar		ar_data[0];
+#define ar_sha		ar_data[0]
+#define ar_spa		ar_data[ARP_HLEN]
+#define ar_tha		ar_data[ARP_HLEN + ARP_PLEN]
+#define ar_tpa		ar_data[ARP_HLEN + ARP_PLEN + ARP_HLEN]
 #if 0
 	uchar		ar_sha[];	/* Sender hardware address	*/
 	uchar		ar_spa[];	/* Sender protocol address	*/
@@ -431,7 +438,7 @@ extern void NetSendPacket(uchar *, int);
 
 /* Transmit UDP packet, performing ARP request if needed */
 extern int	NetSendUDPPacket(uchar *ether, IPaddr_t dest, int dport,
-			int sport, int len);
+			int sport, int payload_len);
 
 /* Processes a received packet */
 extern void	NetReceive(volatile uchar *, int);
diff --git a/net/arp.c b/net/arp.c
index 96ffb85..456decd 100644
--- a/net/arp.c
+++ b/net/arp.c
@@ -63,16 +63,16 @@ void ArpRequest(void)
 
 	arp->ar_hrd = htons(ARP_ETHER);
 	arp->ar_pro = htons(PROT_IP);
-	arp->ar_hln = 6;
-	arp->ar_pln = 4;
+	arp->ar_hln = ARP_HLEN;
+	arp->ar_pln = ARP_PLEN;
 	arp->ar_op = htons(ARPOP_REQUEST);
 
 	/* source ET addr */
-	memcpy(&arp->ar_data[0], NetOurEther, 6);
+	memcpy(&arp->ar_sha, NetOurEther, ARP_HLEN);
 	/* source IP addr */
-	NetWriteIP((uchar *) &arp->ar_data[6], NetOurIP);
+	NetWriteIP(&arp->ar_spa, NetOurIP);
 	/* dest ET addr = 0 */
-	memset(&arp->ar_data[10], '\0', 6);
+	memset(&arp->ar_tha, 0, ARP_HLEN);
 	if ((NetArpWaitPacketIP & NetOurSubnetMask) !=
 	    (NetOurIP & NetOurSubnetMask)) {
 		if (NetOurGatewayIP == 0) {
@@ -85,7 +85,7 @@ void ArpRequest(void)
 		NetArpWaitReplyIP = NetArpWaitPacketIP;
 	}
 
-	NetWriteIP((uchar *) &arp->ar_data[16], NetArpWaitReplyIP);
+	NetWriteIP(&arp->ar_tpa, NetArpWaitReplyIP);
 	(void) eth_send(NetTxPacket, (pkt - NetTxPacket) + ARP_HDR_SIZE);
 }
 
@@ -116,7 +116,7 @@ void ArpTimeoutCheck(void)
 void ArpReceive(struct Ethernet_t *et, struct IP_UDP_t *ip, int len)
 {
 	struct ARP_t *arp;
-	IPaddr_t tmp;
+	IPaddr_t reply_ip_addr;
 	uchar *pkt;
 
 	/*
@@ -139,15 +139,15 @@ void ArpReceive(struct Ethernet_t *et, struct IP_UDP_t *ip, int len)
 		return;
 	if (ntohs(arp->ar_pro) != PROT_IP)
 		return;
-	if (arp->ar_hln != 6)
+	if (arp->ar_hln != ARP_HLEN)
 		return;
-	if (arp->ar_pln != 4)
+	if (arp->ar_pln != ARP_PLEN)
 		return;
 
 	if (NetOurIP == 0)
 		return;
 
-	if (NetReadIP(&arp->ar_data[16]) != NetOurIP)
+	if (NetReadIP(&arp->ar_tpa) != NetOurIP)
 		return;
 
 	switch (ntohs(arp->ar_op)) {
@@ -157,10 +157,10 @@ void ArpReceive(struct Ethernet_t *et, struct IP_UDP_t *ip, int len)
 		pkt = (uchar *)et;
 		pkt += NetSetEther(pkt, et->et_src, PROT_ARP);
 		arp->ar_op = htons(ARPOP_REPLY);
-		memcpy(&arp->ar_data[10], &arp->ar_data[0], 6);
-		NetCopyIP(&arp->ar_data[16], &arp->ar_data[6]);
-		memcpy(&arp->ar_data[0], NetOurEther, 6);
-		NetCopyIP(&arp->ar_data[6], &NetOurIP);
+		memcpy(&arp->ar_tha, &arp->ar_sha, ARP_HLEN);
+		NetCopyIP(&arp->ar_tpa, &arp->ar_spa);
+		memcpy(&arp->ar_sha, NetOurEther, ARP_HLEN);
+		NetCopyIP(&arp->ar_spa, &NetOurIP);
 		(void) eth_send((uchar *)et,
 				(pkt - (uchar *)et) + ARP_HDR_SIZE);
 		return;
@@ -173,28 +173,28 @@ void ArpReceive(struct Ethernet_t *et, struct IP_UDP_t *ip, int len)
 #ifdef CONFIG_KEEP_SERVERADDR
 		if (NetServerIP == NetArpWaitPacketIP) {
 			char buf[20];
-			sprintf(buf, "%pM", arp->ar_data);
+			sprintf(buf, "%pM", arp->ar_sha);
 			setenv("serveraddr", buf);
 		}
 #endif
 
-		tmp = NetReadIP(&arp->ar_data[6]);
+		reply_ip_addr = NetReadIP(&arp->ar_spa);
 
 		/* matched waiting packet's address */
-		if (tmp == NetArpWaitReplyIP) {
+		if (reply_ip_addr == NetArpWaitReplyIP) {
 			debug("Got ARP REPLY, set eth addr (%pM)\n",
 				arp->ar_data);
 
 			/* save address for later use */
 			memcpy(NetArpWaitPacketMAC,
-			       &arp->ar_data[0], 6);
+				&arp->ar_sha, ARP_HLEN);
 
 #ifdef CONFIG_NETCONSOLE
 			NetGetHandler()(0, 0, 0, 0, 0);
 #endif
 			/* modify header, and transmit it */
 			memcpy(((struct Ethernet_t *)NetArpWaitTxPacket)->
-				et_dest, NetArpWaitPacketMAC, 6);
+				et_dest, NetArpWaitPacketMAC, ARP_HLEN);
 			(void) eth_send(NetArpWaitTxPacket,
 					NetArpWaitTxPacketSize);
 
diff --git a/net/bootp.c b/net/bootp.c
index e95419f..2be8083 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -73,7 +73,7 @@ static int BootpCheckPkt(uchar *pkt, unsigned dest, unsigned src, unsigned len)
 
 	if (dest != PORT_BOOTPC || src != PORT_BOOTPS)
 		retval = -1;
-	else if (len < sizeof(struct Bootp_t) - OPT_SIZE)
+	else if (len < sizeof(struct Bootp_t) - OPT_FIELD_SIZE)
 		retval = -2;
 	else if (bp->bp_op != OP_BOOTREQUEST &&
 			bp->bp_op != OP_BOOTREPLY &&
@@ -369,8 +369,8 @@ static int DhcpExtended(u8 *e, int message_type, IPaddr_t ServerID,
 
 	*e++ = 57;		/* Maximum DHCP Message Size */
 	*e++ = 2;
-	*e++ = (576 - 312 + OPT_SIZE) >> 8;
-	*e++ = (576 - 312 + OPT_SIZE) & 0xff;
+	*e++ = (576 - 312 + OPT_FIELD_SIZE) >> 8;
+	*e++ = (576 - 312 + OPT_FIELD_SIZE) & 0xff;
 
 	if (ServerID) {
 		int tmp = ntohl(ServerID);
@@ -520,8 +520,8 @@ static int BootpExtended(u8 *e)
 
 	*e++ = 57;		/* Maximum DHCP Message Size */
 	*e++ = 2;
-	*e++ = (576 - 312 + OPT_SIZE) >> 16;
-	*e++ = (576 - 312 + OPT_SIZE) & 0xff;
+	*e++ = (576 - 312 + OPT_FIELD_SIZE) >> 16;
+	*e++ = (576 - 312 + OPT_FIELD_SIZE) & 0xff;
 #endif
 
 #if defined(CONFIG_BOOTP_SUBNETMASK)
diff --git a/net/bootp.h b/net/bootp.h
index 1cf9a02..ecbcc4d 100644
--- a/net/bootp.h
+++ b/net/bootp.h
@@ -20,13 +20,13 @@
  */
 #if defined(CONFIG_CMD_DHCP)
 /* Minimum DHCP Options size per RFC2131 - results in 576 byte pkt */
-#define OPT_SIZE 312
+#define OPT_FIELD_SIZE 312
 #if defined(CONFIG_BOOTP_VENDOREX)
 extern u8 *dhcp_vendorex_prep(u8 *e); /*rtn new e after add own opts. */
 extern u8 *dhcp_vendorex_proc(u8 *e); /*rtn next e if mine,else NULL  */
 #endif
 #else
-#define OPT_SIZE 64
+#define OPT_FIELD_SIZE 64
 #endif
 
 struct Bootp_t {
@@ -48,11 +48,10 @@ struct Bootp_t {
 	uchar		bp_chaddr[16];	/* Client hardware address	*/
 	char		bp_sname[64];	/* Server host name		*/
 	char		bp_file[128];	/* Boot file name		*/
-	char		bp_vend[OPT_SIZE];	/* Vendor information	*/
+	char		bp_vend[OPT_FIELD_SIZE]; /* Vendor information	*/
 };
 
 #define BOOTP_HDR_SIZE	sizeof(struct Bootp_t)
-#define BOOTP_SIZE	(ETHER_HDR_SIZE + IP_UDP_HDR_SIZE + BOOTP_HDR_SIZE)
 
 /**********************************************************************/
 /*
diff --git a/net/cdp.c b/net/cdp.c
index d617f18..38b79bd 100644
--- a/net/cdp.c
+++ b/net/cdp.c
@@ -245,7 +245,7 @@ CDPDummyHandler(uchar *pkt, unsigned dest, IPaddr_t sip, unsigned src,
 }
 
 void
-CDPHandler(const uchar *pkt, unsigned len)
+CDPReceive(const uchar *pkt, unsigned len)
 {
 	const uchar *t;
 	const ushort *ss;
diff --git a/net/cdp.h b/net/cdp.h
index fef744e..88191c4 100644
--- a/net/cdp.h
+++ b/net/cdp.h
@@ -12,7 +12,7 @@
 #define __CDP_H__
 
 void CDPStart(void);
-void CDPHandler(const uchar *pkt, unsigned len);
+void CDPReceive(const uchar *pkt, unsigned len);
 
 #endif /* __CDP_H__ */
 
diff --git a/net/net.c b/net/net.c
index 023802d..9bf74d5 100644
--- a/net/net.c
+++ b/net/net.c
@@ -75,32 +75,32 @@
 
 
 #include <common.h>
-#include <watchdog.h>
 #include <command.h>
 #include <net.h>
-#include "arp.h"
-#include "bootp.h"
-#include "tftp.h"
-#ifdef CONFIG_CMD_RARP
-#include "rarp.h"
-#endif
-#include "nfs.h"
-#ifdef CONFIG_STATUS_LED
+#if defined(CONFIG_STATUS_LED)
 #include <status_led.h>
 #include <miiphy.h>
 #endif
-#if defined(CONFIG_CMD_SNTP)
-#include "sntp.h"
-#endif
+#include <watchdog.h>
+#include "arp.h"
+#include "bootp.h"
 #if defined(CONFIG_CMD_CDP)
 #include "cdp.h"
 #endif
 #if defined(CONFIG_CMD_DNS)
 #include "dns.h"
 #endif
+#include "nfs.h"
 #if defined(CONFIG_CMD_PING)
 #include "ping.h"
 #endif
+#if defined(CONFIG_CMD_RARP)
+#include "rarp.h"
+#endif
+#if defined(CONFIG_CMD_SNTP)
+#include "sntp.h"
+#endif
+#include "tftp.h"
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -598,7 +598,8 @@ NetSendPacket(uchar *pkt, int len)
 }
 
 int
-NetSendUDPPacket(uchar *ether, IPaddr_t dest, int dport, int sport, int len)
+NetSendUDPPacket(uchar *ether, IPaddr_t dest, int dport, int sport,
+		int payload_len)
 {
 	uchar *pkt;
 
@@ -624,14 +625,14 @@ NetSendUDPPacket(uchar *ether, IPaddr_t dest, int dport, int sport, int len)
 		pkt = NetArpWaitTxPacket;
 		pkt += NetSetEther(pkt, NetArpWaitPacketMAC, PROT_IP);
 
-		NetSetIP(pkt, dest, dport, sport, len);
+		NetSetIP(pkt, dest, dport, sport, payload_len);
 		memcpy(pkt + IP_UDP_HDR_SIZE, (uchar *)NetTxPacket +
 		       (pkt - (uchar *)NetArpWaitTxPacket) +
-		       IP_UDP_HDR_SIZE, len);
+		       IP_UDP_HDR_SIZE, payload_len);
 
 		/* size of the waiting packet */
 		NetArpWaitTxPacketSize = (pkt - NetArpWaitTxPacket) +
-			IP_UDP_HDR_SIZE + len;
+			IP_UDP_HDR_SIZE + payload_len;
 
 		/* and do the ARP request */
 		NetArpWaitTry = 1;
@@ -644,8 +645,9 @@ NetSendUDPPacket(uchar *ether, IPaddr_t dest, int dport, int sport, int len)
 
 	pkt = (uchar *)NetTxPacket;
 	pkt += NetSetEther(pkt, ether, PROT_IP);
-	NetSetIP(pkt, dest, dport, sport, len);
-	eth_send(NetTxPacket, (pkt - NetTxPacket) + IP_UDP_HDR_SIZE + len);
+	NetSetIP(pkt, dest, dport, sport, payload_len);
+	eth_send(NetTxPacket, (pkt - NetTxPacket) + IP_UDP_HDR_SIZE +
+		payload_len);
 
 	return 0;	/* transmitted */
 }
@@ -859,9 +861,9 @@ NetReceive(volatile uchar *inpkt, int len)
 {
 	struct Ethernet_t *et;
 	struct IP_UDP_t	*ip;
-	IPaddr_t tmp;
+	IPaddr_t dst_ip;
 	IPaddr_t src_ip;
-	int	x;
+	int eth_proto;
 #if defined(CONFIG_CMD_CDP)
 	int iscdp;
 #endif
@@ -896,20 +898,21 @@ NetReceive(volatile uchar *inpkt, int len)
 	if (mynvlanid == (ushort)-1)
 		mynvlanid = VLAN_NONE;
 
-	x = ntohs(et->et_protlen);
+	eth_proto = ntohs(et->et_protlen);
 
 	debug("packet received\n");
 
-	if (x < 1514) {
+	if (eth_proto < 1514) {
 		/*
-		 *	Got a 802 packet.  Check the other protocol field.
+		 *	Got a 802.2 packet.  Check the other protocol field.
+		 *	XXX VLAN over 802.2+SNAP not implemented!
 		 */
-		x = ntohs(et->et_prot);
+		eth_proto = ntohs(et->et_prot);
 
 		ip = (struct IP_UDP_t *)(NetRxPacket + E802_HDR_SIZE);
 		len -= E802_HDR_SIZE;
 
-	} else if (x != PROT_VLAN) {	/* normal packet */
+	} else if (eth_proto != PROT_VLAN) {	/* normal packet */
 		ip = (struct IP_UDP_t *)(NetRxPacket + ETHER_HDR_SIZE);
 		len -= ETHER_HDR_SIZE;
 
@@ -932,17 +935,17 @@ NetReceive(volatile uchar *inpkt, int len)
 
 		cti = ntohs(vet->vet_tag);
 		vlanid = cti & VLAN_IDMASK;
-		x = ntohs(vet->vet_type);
+		eth_proto = ntohs(vet->vet_type);
 
 		ip = (struct IP_UDP_t *)(NetRxPacket + VLAN_ETHER_HDR_SIZE);
 		len -= VLAN_ETHER_HDR_SIZE;
 	}
 
-	debug("Receive from protocol 0x%x\n", x);
+	debug("Receive from protocol 0x%x\n", eth_proto);
 
 #if defined(CONFIG_CMD_CDP)
 	if (iscdp) {
-		CDPHandler((uchar *)ip, len);
+		CDPReceive((uchar *)ip, len);
 		return;
 	}
 #endif
@@ -955,7 +958,7 @@ NetReceive(volatile uchar *inpkt, int len)
 			return;
 	}
 
-	switch (x) {
+	switch (eth_proto) {
 
 	case PROT_ARP:
 		ArpReceive(et, ip, len);
@@ -994,10 +997,10 @@ NetReceive(volatile uchar *inpkt, int len)
 			return;
 		}
 		/* If it is not for us, ignore it */
-		tmp = NetReadIP(&ip->ip_dst);
-		if (NetOurIP && tmp != NetOurIP && tmp != 0xFFFFFFFF) {
+		dst_ip = NetReadIP(&ip->ip_dst);
+		if (NetOurIP && dst_ip != NetOurIP && dst_ip != 0xFFFFFFFF) {
 #ifdef CONFIG_MCAST_TFTP
-			if (Mcast_addr != tmp)
+			if (Mcast_addr != dst_ip)
 #endif
 				return;
 		}
-- 
1.6.0.2



More information about the U-Boot mailing list