From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:47162) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RlHiX-0001fA-UR for qemu-devel@nongnu.org; Thu, 12 Jan 2012 05:17:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RlHiR-0001JO-S1 for qemu-devel@nongnu.org; Thu, 12 Jan 2012 05:17:09 -0500 Received: from mailout1.w1.samsung.com ([210.118.77.11]:34545) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RlHiR-0001JF-Ni for qemu-devel@nongnu.org; Thu, 12 Jan 2012 05:17:03 -0500 Received: from euspt2 (mailout1.w1.samsung.com [210.118.77.11]) by mailout1.w1.samsung.com (iPlanet Messaging Server 5.2 Patch 2 (built Jul 14 2004)) with ESMTP id <0LXO00I7AKKB73@mailout1.w1.samsung.com> for qemu-devel@nongnu.org; Thu, 12 Jan 2012 10:16:59 +0000 (GMT) Received: from [106.109.8.162] by spt2.w1.samsung.com (iPlanet Messaging Server 5.2 Patch 2 (built Jul 14 2004)) with ESMTPA id <0LXO002NQKK85Q@spt2.w1.samsung.com> for qemu-devel@nongnu.org; Thu, 12 Jan 2012 10:16:59 +0000 (GMT) Date: Thu, 12 Jan 2012 14:16:56 +0400 From: Mitsyanko Igor In-reply-to: <1324332111-29059-1-git-send-email-peter.maydell@linaro.org> Message-id: <4F0EB318.3070209@samsung.com> MIME-version: 1.0 Content-type: text/plain; charset=ISO-8859-1; format=flowed Content-transfer-encoding: 7BIT References: <1324332111-29059-1-git-send-email-peter.maydell@linaro.org> Subject: Re: [Qemu-devel] [PATCH] hw/lan9118: Add save/load support Reply-To: i.mitsyanko@samsung.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-devel@nongnu.org, patches@linaro.org On 12/20/2011 02:01 AM, Peter Maydell wrote: > Implement save/load for the LAN9118. > > Signed-off-by: Peter Maydell > --- > 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: i.mitsyanko@samsung.com