diff mbox series

[v3,04/14] dp8393x: Have dp8393x_receive() return the packet size

Message ID a582b4412af479e0830d1ef0005a91a98bfc56cc.1579474761.git.fthain@telegraphics.com.au
State New
Headers show
Series Fixes for DP8393X SONIC device emulation | expand

Commit Message

Finn Thain Jan. 19, 2020, 10:59 p.m. UTC
This function re-uses its 'size' argument as a scratch variable.
Instead, declare a local 'size' variable for that purpose so that the
function result doesn't get messed up.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/net/dp8393x.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Philippe Mathieu-Daudé Jan. 28, 2020, 11:03 a.m. UTC | #1
On 1/19/20 11:59 PM, Finn Thain wrote:
> This function re-uses its 'size' argument as a scratch variable.
> Instead, declare a local 'size' variable for that purpose so that the
> function result doesn't get messed up.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Tested-by: Laurent Vivier <laurent@vivier.eu>
> ---
>   hw/net/dp8393x.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 2d2ace2549..ece72cbf42 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -757,20 +757,21 @@ static int dp8393x_receive_filter(dp8393xState *s, const uint8_t * buf,
>   }
>   
>   static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
> -                               size_t size)
> +                               size_t pkt_size)
>   {
>       dp8393xState *s = qemu_get_nic_opaque(nc);
>       int packet_type;
>       uint32_t available, address;
> -    int width, rx_len = size;
> +    int width, rx_len = pkt_size;
>       uint32_t checksum;
> +    int size;
>   
>       width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
>   
>       s->regs[SONIC_RCR] &= ~(SONIC_RCR_PRX | SONIC_RCR_LBK | SONIC_RCR_FAER |
>           SONIC_RCR_CRCR | SONIC_RCR_LPKT | SONIC_RCR_BC | SONIC_RCR_MC);
>   
> -    packet_type = dp8393x_receive_filter(s, buf, size);
> +    packet_type = dp8393x_receive_filter(s, buf, pkt_size);
>       if (packet_type < 0) {
>           DPRINTF("packet not for netcard\n");
>           return -1;
> @@ -864,7 +865,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
>       /* Done */
>       dp8393x_update_irq(s);
>   
> -    return size;
> +    return pkt_size;
>   }
>   
>   static void dp8393x_reset(DeviceState *dev)
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Philippe Mathieu-Daudé Jan. 28, 2020, 11:04 a.m. UTC | #2
On Tue, Jan 28, 2020 at 12:03 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
> On 1/19/20 11:59 PM, Finn Thain wrote:
> > This function re-uses its 'size' argument as a scratch variable.
> > Instead, declare a local 'size' variable for that purpose so that the
> > function result doesn't get messed up.
> >
> > Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Tested-by: Laurent Vivier <laurent@vivier.eu>
> > ---
> >   hw/net/dp8393x.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> > index 2d2ace2549..ece72cbf42 100644
> > --- a/hw/net/dp8393x.c
> > +++ b/hw/net/dp8393x.c
> > @@ -757,20 +757,21 @@ static int dp8393x_receive_filter(dp8393xState *s, const uint8_t * buf,
> >   }
> >
> >   static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
> > -                               size_t size)
> > +                               size_t pkt_size)
> >   {
> >       dp8393xState *s = qemu_get_nic_opaque(nc);
> >       int packet_type;
> >       uint32_t available, address;
> > -    int width, rx_len = size;
> > +    int width, rx_len = pkt_size;
> >       uint32_t checksum;
> > +    int size;
> >
> >       width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
> >
> >       s->regs[SONIC_RCR] &= ~(SONIC_RCR_PRX | SONIC_RCR_LBK | SONIC_RCR_FAER |
> >           SONIC_RCR_CRCR | SONIC_RCR_LPKT | SONIC_RCR_BC | SONIC_RCR_MC);
> >
> > -    packet_type = dp8393x_receive_filter(s, buf, size);
> > +    packet_type = dp8393x_receive_filter(s, buf, pkt_size);
> >       if (packet_type < 0) {
> >           DPRINTF("packet not for netcard\n");
> >           return -1;
> > @@ -864,7 +865,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
> >       /* Done */
> >       dp8393x_update_irq(s);
> >
> > -    return size;
> > +    return pkt_size;
> >   }
> >
> >   static void dp8393x_reset(DeviceState *dev)
> >
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

One R-b tag is enough btw ;)
diff mbox series

Patch

diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
index 2d2ace2549..ece72cbf42 100644
--- a/hw/net/dp8393x.c
+++ b/hw/net/dp8393x.c
@@ -757,20 +757,21 @@  static int dp8393x_receive_filter(dp8393xState *s, const uint8_t * buf,
 }
 
 static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
-                               size_t size)
+                               size_t pkt_size)
 {
     dp8393xState *s = qemu_get_nic_opaque(nc);
     int packet_type;
     uint32_t available, address;
-    int width, rx_len = size;
+    int width, rx_len = pkt_size;
     uint32_t checksum;
+    int size;
 
     width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1;
 
     s->regs[SONIC_RCR] &= ~(SONIC_RCR_PRX | SONIC_RCR_LBK | SONIC_RCR_FAER |
         SONIC_RCR_CRCR | SONIC_RCR_LPKT | SONIC_RCR_BC | SONIC_RCR_MC);
 
-    packet_type = dp8393x_receive_filter(s, buf, size);
+    packet_type = dp8393x_receive_filter(s, buf, pkt_size);
     if (packet_type < 0) {
         DPRINTF("packet not for netcard\n");
         return -1;
@@ -864,7 +865,7 @@  static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
     /* Done */
     dp8393x_update_irq(s);
 
-    return size;
+    return pkt_size;
 }
 
 static void dp8393x_reset(DeviceState *dev)