[v2] e1000: cleanup process_tx_desc

Coverity complains about two overruns in process_tx_desc(). The
complaints are false positives, but we might as well eliminate
them. The problem is that "hdr" is defined as an unsigned int,
but then used to offset an array of size 65536, and another of
size 256 bytes. hdr will actually never be greater than 255
though, as it's assigned only once and to the value of
tp->hdr_len, which is an uint8_t. This patch simply gets rid of
hdr, replacing it with tp->hdr_len, which makes it consistent
with all other tp member use in the function.

v2:
 - also cleanup coding style issues in the touched lines

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 hw/net/e1000.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

On Tue, Jun 04, 2013 at 10:49:48AM +0200, Andrew Jones wrote:
> Coverity complains about two overruns in process_tx_desc(). The
> complaints are false positives, but we might as well eliminate
> them. The problem is that "hdr" is defined as an unsigned int,
> but then used to offset an array of size 65536, and another of
> size 256 bytes. hdr will actually never be greater than 255
> though, as it's assigned only once and to the value of
> tp->hdr_len, which is an uint8_t. This patch simply gets rid of
> hdr, replacing it with tp->hdr_len, which makes it consistent
> with all other tp member use in the function.
> 
> v2:
>  - also cleanup coding style issues in the touched lines
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>

I've picked this one up.
Thanks,
    MST

> ---
>  hw/net/e1000.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index e6f46f0c511e8..620f947134780 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -556,7 +556,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
>      uint32_t txd_lower = le32_to_cpu(dp->lower.data);
>      uint32_t dtype = txd_lower & (E1000_TXD_CMD_DEXT | E1000_TXD_DTYP_D);
>      unsigned int split_size = txd_lower & 0xffff, bytes, sz, op;
> -    unsigned int msh = 0xfffff, hdr = 0;
> +    unsigned int msh = 0xfffff;
>      uint64_t addr;
>      struct e1000_context_desc *xp = (struct e1000_context_desc *)dp;
>      struct e1000_tx *tp = &s->tx;
> @@ -603,8 +603,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
>          
>      addr = le64_to_cpu(dp->buffer_addr);
>      if (tp->tse && tp->cptse) {
> -        hdr = tp->hdr_len;
> -        msh = hdr + tp->mss;
> +        msh = tp->hdr_len + tp->mss;
>          do {
>              bytes = split_size;
>              if (tp->size + bytes > msh)
> @@ -612,14 +611,16 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
>  
>              bytes = MIN(sizeof(tp->data) - tp->size, bytes);
>              pci_dma_read(&s->dev, addr, tp->data + tp->size, bytes);
> -            if ((sz = tp->size + bytes) >= hdr && tp->size < hdr)
> -                memmove(tp->header, tp->data, hdr);
> +            sz = tp->size + bytes;
> +            if (sz >= tp->hdr_len && tp->size < tp->hdr_len) {
> +                memmove(tp->header, tp->data, tp->hdr_len);
> +            }
>              tp->size = sz;
>              addr += bytes;
>              if (sz == msh) {
>                  xmit_seg(s);
> -                memmove(tp->data, tp->header, hdr);
> -                tp->size = hdr;
> +                memmove(tp->data, tp->header, tp->hdr_len);
> +                tp->size = tp->hdr_len;
>              }
>          } while (split_size -= bytes);
>      } else if (!tp->tse && tp->cptse) {
> @@ -633,8 +634,9 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
>  
>      if (!(txd_lower & E1000_TXD_CMD_EOP))
>          return;
> -    if (!(tp->tse && tp->cptse && tp->size < hdr))
> +    if (!(tp->tse && tp->cptse && tp->size < tp->hdr_len)) {
>          xmit_seg(s);
> +    }
>      tp->tso_frames = 0;
>      tp->sum_needed = 0;
>      tp->vlan_needed = 0;
> -- 
> 1.8.1.4