Re: [Qemu-devel] [PATCH V7 5/6] Adding packet abstraction for VMWARE network devices

[Stefan Hajnoczi]

On Fri, Nov 16, 2012 at 2:55 PM, Dmitry Fleytman <address@hidden> wrote:
> +/*=============================================================================
> + 
> *=============================================================================
> + *
> + *                            TX CODE
> + *
> + 
> *=============================================================================
> + 
> *===========================================================================*/

Is this huge comment box a sign that the code should be split into a
foo_tx.c and an foo_rx.c file?

> +size_t vmxnet_tx_pkt_send(VmxnetTxPktH pkt, NetClientState *vc)

'vc' is an old name that was used for VLANClientState.  The struct has
since been renamed to NetClientState and the rest of QEMU uses 'nc'
instead of 'vc'.

> +/* tx module context handle */
> +typedef void *VmxnetTxPktH;

Forward-declaring the struct is nicer:

typedef struct VmxnetTxPkt VmxnetTxPkt;

The definition of VmxnetTxPkt is still hidden from the caller but you
avoid the void* and casting.  In vmxnet_pkt.c define using:

struct VmxnetTxPkt {
    ...
};

> diff --git a/hw/vmxnet_utils.c b/hw/vmxnet_utils.c
> index 9ba045f..56fda9f 100644
> --- a/hw/vmxnet_utils.c
> +++ b/hw/vmxnet_utils.c
> @@ -6,9 +6,9 @@
>   * Developed by Daynix Computing LTD (http://www.daynix.com)
>   *
>   * Authors:
> - * Dmitry Fleytman <address@hidden>
> - * Tamir Shomer <address@hidden>
> - * Yan Vugenfirer <address@hidden>
> + * Dmitry Fleytman <address@hidden>
> + * Tamir Shomer <address@hidden>
> + * Yan Vugenfirer <address@hidden>
>   *
>   * This work is licensed under the terms of the GNU GPL, version 2 or later.
>   * See the COPYING file in the top-level directory.

Please squash these fixes into the previous patch.

> diff --git a/hw/vmxnet_utils.h b/hw/vmxnet_utils.h
> index 7fd9a01..fac3b7b 100644
> --- a/hw/vmxnet_utils.h
> +++ b/hw/vmxnet_utils.h

Please squash these fixes into the previous patch.