From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1W39BY-0004t5-IJ for mharc-qemu-trivial@gnu.org; Tue, 14 Jan 2014 13:58:00 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47336) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W39BR-0004l0-RJ for qemu-trivial@nongnu.org; Tue, 14 Jan 2014 13:57:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W39BM-0002qD-VY for qemu-trivial@nongnu.org; Tue, 14 Jan 2014 13:57:53 -0500 Received: from mx1.redhat.com ([209.132.183.28]:13565) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W39BC-0002mU-Ok; Tue, 14 Jan 2014 13:57:39 -0500 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s0EITwZW020733 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 14 Jan 2014 13:29:59 -0500 Received: from localhost (ovpn-113-136.phx2.redhat.com [10.3.113.136]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s0EITv0p025030; Tue, 14 Jan 2014 13:29:57 -0500 Date: Tue, 14 Jan 2014 13:29:56 -0500 From: Luiz Capitulino To: Michael Tokarev Message-ID: <20140114132956.1c21c68c@redhat.com> In-Reply-To: <52D56E5B.9000001@msgid.tls.msk.ru> References: <20140109095816.409cd2a9@redhat.com> <52D56E5B.9000001@msgid.tls.msk.ru> Organization: Red Hat Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: qemu-trivial , Peter Crosthwaite , qemu-devel , mst@redhat.com Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH] virtio-balloon: don't hardcode config size value X-BeenThere: qemu-trivial@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Jan 2014 18:57:58 -0000 On Tue, 14 Jan 2014 21:05:31 +0400 Michael Tokarev wrote: > 09.01.2014 18:58, Luiz Capitulino wrote: > > Use sizeof(strucy virtio_balloon_config) instead. > > > > --- a/hw/virtio/virtio-balloon.c > > +++ b/hw/virtio/virtio-balloon.c > > @@ -263,7 +263,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data) > > config.num_pages = cpu_to_le32(dev->num_pages); > > config.actual = cpu_to_le32(dev->actual); > > > > - memcpy(config_data, &config, 8); > > + memcpy(config_data, &config, sizeof(struct virtio_balloon_config)); > > I'm not sure any of those is better than another. No duplication, no risk of changing virtio_balloon_config and forgetting about changing all the memcpys out there (which is exactly what happened to me). This is also what the other devices do. > This is a published guest <=> host interface, the config _must_ be 8 bytes > long and must contain 2 4-byte words in it. That's not changed by this patch. > We may use assert(sizeof(struct virtio_balloon_config) == 8) somewhere, > but to my taste it is a bit overkill. No? > > Thanks, > > /mjt > From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47274) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W39BI-0004b0-3D for qemu-devel@nongnu.org; Tue, 14 Jan 2014 13:57:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W39BD-0002mr-70 for qemu-devel@nongnu.org; Tue, 14 Jan 2014 13:57:44 -0500 Date: Tue, 14 Jan 2014 13:29:56 -0500 From: Luiz Capitulino Message-ID: <20140114132956.1c21c68c@redhat.com> In-Reply-To: <52D56E5B.9000001@msgid.tls.msk.ru> References: <20140109095816.409cd2a9@redhat.com> <52D56E5B.9000001@msgid.tls.msk.ru> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] virtio-balloon: don't hardcode config size value List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev Cc: qemu-trivial , Peter Crosthwaite , qemu-devel , mst@redhat.com On Tue, 14 Jan 2014 21:05:31 +0400 Michael Tokarev wrote: > 09.01.2014 18:58, Luiz Capitulino wrote: > > Use sizeof(strucy virtio_balloon_config) instead. > > > > --- a/hw/virtio/virtio-balloon.c > > +++ b/hw/virtio/virtio-balloon.c > > @@ -263,7 +263,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data) > > config.num_pages = cpu_to_le32(dev->num_pages); > > config.actual = cpu_to_le32(dev->actual); > > > > - memcpy(config_data, &config, 8); > > + memcpy(config_data, &config, sizeof(struct virtio_balloon_config)); > > I'm not sure any of those is better than another. No duplication, no risk of changing virtio_balloon_config and forgetting about changing all the memcpys out there (which is exactly what happened to me). This is also what the other devices do. > This is a published guest <=> host interface, the config _must_ be 8 bytes > long and must contain 2 4-byte words in it. That's not changed by this patch. > We may use assert(sizeof(struct virtio_balloon_config) == 8) somewhere, > but to my taste it is a bit overkill. No? > > Thanks, > > /mjt >