diff mbox

[U-Boot,v2,21/21] net: cosmetic: Replace magic numbers in arp.c with constants

Message ID 1332891772-17279-22-git-send-email-joe.hershberger@ni.com
State Superseded
Delegated to: Joe Hershberger
Headers show

Commit Message

Joe Hershberger March 27, 2012, 11:42 p.m. UTC 
Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
Cc: Joe Hershberger <joe.hershberger@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Mike Frysinger <vapier@gentoo.org>
---
Changes for v2:
   - Split from "Improve variable names and code readability"

 include/net.h |    6 ++++++
 net/arp.c     |   34 +++++++++++++++++-----------------
 2 files changed, 23 insertions(+), 17 deletions(-)

Comments

Simon Glass April 27, 2012, 6:26 a.m. UTC | #1 
Hi Joe,

On Wed, Mar 28, 2012 at 12:42 PM, Joe Hershberger <joe.hershberger@ni.com>wrote:

> Signed-off-by: Joe Hershberger <joe.hershberger@ni.com>
> Cc: Joe Hershberger <joe.hershberger@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Mike Frysinger <vapier@gentoo.org>
> ---
> Changes for v2:
>   - Split from "Improve variable names and code readability"
>
>  include/net.h |    6 ++++++
>  net/arp.c     |   34 +++++++++++++++++-----------------
>  2 files changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/include/net.h b/include/net.h
> index 3314b4e..bb41f5e 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -258,7 +258,9 @@ struct ARP_hdr {
>  #   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 */
> @@ -272,6 +274,10 @@ struct ARP_hdr {
>         * 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      */
> diff --git a/net/arp.c b/net/arp.c
> index f994e7d..6e3d7ab 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);
>

Did you mean to change this to 0? I prefer '\0\ but I might be the only one.


>        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);
>  }
>
> @@ -139,15 +139,15 @@ void ArpReceive(struct Ethernet_hdr *et, struct
> IP_UDP_hdr *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_hdr *et, struct
> IP_UDP_hdr *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,12 +173,12 @@ void ArpReceive(struct Ethernet_hdr *et, struct
> IP_UDP_hdr *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
>
> -               reply_ip_addr = NetReadIP(&arp->ar_data[6]);
> +               reply_ip_addr = NetReadIP(&arp->ar_spa);
>
>                /* matched waiting packet's address */
>                if (reply_ip_addr == NetArpWaitReplyIP) {
> @@ -187,14 +187,14 @@ void ArpReceive(struct Ethernet_hdr *et, struct
> IP_UDP_hdr *ip, int len)
>
>                        /* 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_hdr *)NetArpWaitTxPacket)->
> -                               et_dest, NetArpWaitPacketMAC, 6);
> +                               et_dest, NetArpWaitPacketMAC, ARP_HLEN);
>                        (void) eth_send(NetArpWaitTxPacket,
>                                        NetArpWaitTxPacketSize);
>
> --
> 1.6.0.2
>
> Regards,
Simon
diff mbox

Patch

diff --git a/include/net.h b/include/net.h
index 3314b4e..bb41f5e 100644
--- a/include/net.h
+++ b/include/net.h
@@ -258,7 +258,9 @@  struct ARP_hdr {
 #   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	*/
@@ -272,6 +274,10 @@  struct ARP_hdr {
 	 * 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	*/
diff --git a/net/arp.c b/net/arp.c
index f994e7d..6e3d7ab 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);
 }
 
@@ -139,15 +139,15 @@  void ArpReceive(struct Ethernet_hdr *et, struct IP_UDP_hdr *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_hdr *et, struct IP_UDP_hdr *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,12 +173,12 @@  void ArpReceive(struct Ethernet_hdr *et, struct IP_UDP_hdr *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
 
-		reply_ip_addr = NetReadIP(&arp->ar_data[6]);
+		reply_ip_addr = NetReadIP(&arp->ar_spa);
 
 		/* matched waiting packet's address */
 		if (reply_ip_addr == NetArpWaitReplyIP) {
@@ -187,14 +187,14 @@  void ArpReceive(struct Ethernet_hdr *et, struct IP_UDP_hdr *ip, int len)
 
 			/* 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_hdr *)NetArpWaitTxPacket)->
-				et_dest, NetArpWaitPacketMAC, 6);
+				et_dest, NetArpWaitPacketMAC, ARP_HLEN);
 			(void) eth_send(NetArpWaitTxPacket,
 					NetArpWaitTxPacketSize);