From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1W3AFT-00011X-27 for mharc-qemu-trivial@gnu.org; Tue, 14 Jan 2014 15:06:07 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34885) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W3AFL-0000jn-17 for qemu-trivial@nongnu.org; Tue, 14 Jan 2014 15:06:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W3AFE-0007OY-Ro for qemu-trivial@nongnu.org; Tue, 14 Jan 2014 15:05:58 -0500 Received: from mx1.redhat.com ([209.132.183.28]:31153) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W3AF1-0007MW-Qc; Tue, 14 Jan 2014 15:05:40 -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 s0EK5cEk005018 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 14 Jan 2014 15:05:38 -0500 Received: from redhat.com (vpn1-4-212.ams2.redhat.com [10.36.4.212]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with SMTP id s0EK5ZYj014101; Tue, 14 Jan 2014 15:05:36 -0500 Date: Tue, 14 Jan 2014 22:05:35 +0200 From: "Michael S. Tsirkin" To: Michael Tokarev Message-ID: <20140114200535.GA31642@redhat.com> References: <20140109095816.409cd2a9@redhat.com> <52D56E5B.9000001@msgid.tls.msk.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <52D56E5B.9000001@msgid.tls.msk.ru> 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 , Luiz Capitulino 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 20:06:05 -0000 On Tue, Jan 14, 2014 at 09:05:31PM +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. > > This is a published guest <=> host interface, the config _must_ be 8 bytes > long and must contain 2 4-byte words in it. no, config can be extended in the future. and hard coded constants are evil. > > We may use assert(sizeof(struct virtio_balloon_config) == 8) somewhere, > but to my taste it is a bit overkill. No? I agree assert like this would be overkill. > Thanks, > > /mjt -- MST From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34849) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W3AF8-0000WG-OQ for qemu-devel@nongnu.org; Tue, 14 Jan 2014 15:05:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W3AF2-0007Me-Mx for qemu-devel@nongnu.org; Tue, 14 Jan 2014 15:05:46 -0500 Date: Tue, 14 Jan 2014 22:05:35 +0200 From: "Michael S. Tsirkin" Message-ID: <20140114200535.GA31642@redhat.com> References: <20140109095816.409cd2a9@redhat.com> <52D56E5B.9000001@msgid.tls.msk.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <52D56E5B.9000001@msgid.tls.msk.ru> 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 , Luiz Capitulino On Tue, Jan 14, 2014 at 09:05:31PM +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. > > This is a published guest <=> host interface, the config _must_ be 8 bytes > long and must contain 2 4-byte words in it. no, config can be extended in the future. and hard coded constants are evil. > > We may use assert(sizeof(struct virtio_balloon_config) == 8) somewhere, > but to my taste it is a bit overkill. No? I agree assert like this would be overkill. > Thanks, > > /mjt -- MST