Re: [PATCH 03/10] dp8393x: Have dp8393x_receive() return the packet size

[Philippe Mathieu-Daudé]

Hi Finn,

On 12/14/19 2:25 AM, 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 <address@hidden>
> ---
>   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 3fd59bc1d4..462f8646e0 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -741,20 +741,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;

I'm tempted to remove rx_len and use pkt_size, pkt_size + 
sizeof(checksum) in place.

Anyway:
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

>       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;
> @@ -848,7 +849,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)