Re: [Qemu-devel] [PATCH] hw/lan9118: Add save/load support

[Mitsyanko Igor]

On 12/20/2011 02:01 AM, Peter Maydell wrote:
> Implement save/load for the LAN9118.
>
> Signed-off-by: Peter Maydell<address@hidden>
> ---
> Does anybody have a nicer solution to the "can't put an enum in a
> VMStateDescription" problem?
>
>   hw/lan9118.c |  126 +++++++++++++++++++++++++++++++++++++++++++++++-----------
>   1 files changed, 103 insertions(+), 23 deletions(-)
>
> diff --git a/hw/lan9118.c b/hw/lan9118.c
> index 7e64c5d..6542a26 100644
> --- a/hw/lan9118.c
> +++ b/hw/lan9118.c
> @@ -136,17 +136,36 @@ enum tx_state {
>   };
Hi Peter, I have a small question. Wouldn't it be better to use uint32_t 
for variables that actually have positive values only? That would make 
code easier to understand. For example in this state

>   typedef struct {
> -    enum tx_state state;
> +    /* state is a tx_state but we can't put enums in VMStateDescriptions. */
> +    uint32_t state;
>       uint32_t cmd_a;
>       uint32_t cmd_b;
> -    int buffer_size;
> -    int offset;
> -    int pad;
> -    int fifo_used;
> -    int len;
> +    int32_t buffer_size;
> +    int32_t offset;
> +    int32_t pad;
> +    int32_t fifo_used;
> +    int32_t len;
>       uint8_t data[2048];
>   } LAN9118Packet;
all member variables (except maybe buffer_size) can have positive values 
only. buffer_size is probably positive-only too because this code

            while (n--) {
                s->txp->data[s->txp->len] = val & 0xff;
                s->txp->len++;
                val >>= 8;
                s->txp->buffer_size--;
            }
looks wrong (I haven't dug dip into it though) and must be
            ...
            while (s->txp->buffer_size && (n--)) {
            ...

--
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: address@hidden