[U-Boot] [PATCH 13/28] net: Refactor bootp packet length computations

[Simon Glass]

Hi Joe,

On Thu, Jan 19, 2012 at 4:53 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: Wolfgang Denk <wd at denx.de>
> ---
>  net/bootp.c |   26 ++++++++++++++------------
>  1 files changed, 14 insertions(+), 12 deletions(-)
>

Refactor why or to what purpose? Perhaps a bit more detail in the
commit message?

> diff --git a/net/bootp.c b/net/bootp.c
> index 0c2af48..0d5f4cf 100644
> --- a/net/bootp.c
> +++ b/net/bootp.c
> @@ -582,7 +582,8 @@ BootpRequest(void)
>  {
>        uchar *pkt, *iphdr;
>        struct Bootp_t *bp;
> -       int ext_len, pktlen, iplen;
> +       int extlen, pktlen, iplen;

Is extlen better than ext_len?

> +       int eth_hdr_size;
>  #ifdef CONFIG_BOOTP_RANDOM_DELAY
>        ulong i, rand_ms;
>  #endif
> @@ -610,7 +611,8 @@ BootpRequest(void)
>        pkt = NetTxPacket;
>        memset((void *)pkt, 0, PKTSIZE);
>
> -       pkt += NetSetEther(pkt, NetBcastAddr, PROT_IP);
> +       eth_hdr_size = NetSetEther(pkt, NetBcastAddr, PROT_IP);
> +       pkt += eth_hdr_size;
>
>        /*
>         * Next line results in incorrect packet size being transmitted,
> @@ -639,9 +641,9 @@ BootpRequest(void)
>
>        /* Request additional information from the BOOTP/DHCP server */
>  #if defined(CONFIG_CMD_DHCP)
> -       ext_len = DhcpExtended((u8 *)bp->bp_vend, DHCP_DISCOVER, 0, 0);
> +       extlen = DhcpExtended((u8 *)bp->bp_vend, DHCP_DISCOVER, 0, 0);
>  #else
> -       ext_len = BootpExtended((u8 *)bp->bp_vend);
> +       extlen = BootpExtended((u8 *)bp->bp_vend);
>  #endif
>
>        /*
> @@ -660,9 +662,8 @@ BootpRequest(void)
>         * Calculate proper packet lengths taking into account the
>         * variable size of the options field
>         */
> -       pktlen = ((int)(pkt-NetTxPacket)) + BOOTP_HDR_SIZE -
> -               sizeof(bp->bp_vend) + ext_len;
> -       iplen = BOOTP_HDR_SIZE - sizeof(bp->bp_vend) + ext_len;
> +       iplen = BOOTP_HDR_SIZE - OPT_FIELD_SIZE + extlen;
> +       pktlen = eth_hdr_size + IP_UDP_HDR_SIZE + iplen;
>        NetSetUDPHeader(iphdr, 0xFFFFFFFFL, PORT_BOOTPS, PORT_BOOTPC, iplen);
>        NetSetTimeout(SELECT_TIMEOUT, BootpTimeout);
>
> @@ -798,13 +799,15 @@ static void DhcpSendRequestPkt(struct Bootp_t *bp_offer)
>        uchar *pkt, *iphdr;
>        struct Bootp_t *bp;
>        int pktlen, iplen, extlen;
> +       int eth_hdr_size;
>        IPaddr_t OfferedIP;
>
>        debug("DhcpSendRequestPkt: Sending DHCPREQUEST\n");
>        pkt = NetTxPacket;
>        memset((void *)pkt, 0, PKTSIZE);
>
> -       pkt += NetSetEther(pkt, NetBcastAddr, PROT_IP);
> +       eth_hdr_size = NetSetEther(pkt, NetBcastAddr, PROT_IP);
> +       pkt += eth_hdr_size;
>
>        iphdr = pkt;    /* We'll need this later to set proper pkt size */
>        pkt += IP_UDP_HDR_SIZE;
> @@ -841,15 +844,14 @@ static void DhcpSendRequestPkt(struct Bootp_t *bp_offer)
>        extlen = DhcpExtended((u8 *)bp->bp_vend, DHCP_REQUEST,
>                NetDHCPServerIP, OfferedIP);
>
> -       pktlen = ((int)(pkt-NetTxPacket)) + BOOTP_HDR_SIZE -
> -               sizeof(bp->bp_vend) + extlen;
> -       iplen = BOOTP_HDR_SIZE - sizeof(bp->bp_vend) + extlen;
> +       iplen = BOOTP_HDR_SIZE - OPT_FIELD_SIZE + extlen;
> +       pktlen = eth_hdr_size + IP_UDP_HDR_SIZE + iplen;
>        NetSetUDPHeader(iphdr, 0xFFFFFFFFL, PORT_BOOTPS, PORT_BOOTPC, iplen);
>
> -       debug("Transmitting DHCPREQUEST packet: len = %d\n", pktlen);
>  #ifdef CONFIG_BOOTP_DHCP_REQUEST_DELAY
>        udelay(CONFIG_BOOTP_DHCP_REQUEST_DELAY);
>  #endif /* CONFIG_BOOTP_DHCP_REQUEST_DELAY */
> +       debug("Transmitting DHCPREQUEST packet: len = %d\n", pktlen);

Did you move this on purpose?

>        NetSendPacket(NetTxPacket, pktlen);
>  }
>
> --
> 1.6.0.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Regards,
Simon