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

[Simon Glass]

Hi Joe,

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

> Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
> Cc: Joe Hershberger <joe.hershberger at gmail.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Mike Frysinger <vapier at 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