virtio-balloon: don't hardcode config size value
PATCH 1/1 virtio-balloon: don't hardcode config size value
Use sizeof(strucy virtio_balloon_config) instead.
---
I have no idea which queue this should go through...
hw/virtio/virtio-balloon.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
hw/virtio/virtio-balloon.c +4 -3
@@ -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 )); } static void virtio_balloon_set_config(VirtIODevice *vdev,
@@ -272,7 +272,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,VirtIOBalloon *dev =VIRTIO_BALLOON (vdev);struct virtio_balloon_config config;uint32_t oldactual = dev->actual; -memcpy (&config, config_data,8 ); +memcpy (&config, config_data,sizeof (struct virtio_balloon_config )); dev->actual =le32_to_cpu (config.actual);if (dev->actual != oldactual) { qemu_balloon_changed(ram_size -
@@ -343,7 +343,8 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)VirtIOBalloon *s =VIRTIO_BALLOON (dev);int ret; -virtio_init (vdev,"virtio-balloon" ,VIRTIO_ID_BALLOON ,8 ); +virtio_init (vdev,"virtio-balloon" ,VIRTIO_ID_BALLOON , +sizeof (struct virtio_balloon_config )); ret =qemu_add_balloon_handler (virtio_balloon_to_target, virtio_balloon_stat, s);
Signature · 1 line
1.8.1.4
Quoted 2 lines · Luiz Capitulino
Use
sizeof(strucy virtio_balloon_config)instead.
"struct".
Quoted 1 line
Signed-off-by Luiz capitulino <[email protected]>
Otherwise:
Quoted 4 lines
---
I have no idea which queue this should go through...
Trivial?
Regards,
Peter
Quoted 39 lines
hw/virtio/virtio-balloon.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)hw/virtio/virtio-balloon.c +4
-4@@ -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 )); } static void virtio_balloon_set_config(VirtIODevice *vdev,@@ -272,7 +272,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,VirtIOBalloon *dev =VIRTIO_BALLOON (vdev);struct virtio_balloon_config config;uint32_t oldactual = dev->actual; -memcpy (&config, config_data,8 ); +memcpy (&config, config_data,sizeof (struct virtio_balloon_config )); dev->actual =le32_to_cpu (config.actual);if (dev->actual != oldactual) { qemu_balloon_changed(ram_size -@@ -343,7 +343,8 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)VirtIOBalloon *s =VIRTIO_BALLOON (dev);int ret; -virtio_init (vdev,"virtio-balloon" ,VIRTIO_ID_BALLOON ,8 ); +virtio_init (vdev,"virtio-balloon" ,VIRTIO_ID_BALLOON , +sizeof (struct virtio_balloon_config )); ret =qemu_add_balloon_handler (virtio_balloon_to_target, virtio_balloon_stat, s); --1.8.1.4
hw/virtio/virtio-balloon.c static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data) · line 266
Quoted 10 lines · Luiz Capitulino
Use
sizeof(strucy virtio_balloon_config)instead.hw/virtio/virtio-balloon.c +1
-1@@ -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.
We may use assert(sizeof(struct virtio_balloon_config) == 8) somewhere, but to my taste it is a bit overkill. No?
Thanks,
/mjt
Quoted 9 lines · On Tue, 14 Jan 2014 21:05:31 +0400 Michael Tokarev <[email protected]> wrote:
Quoted 8 lines · Luiz Capitulino
Use
sizeof(strucy virtio_balloon_config)instead.hw/virtio/virtio-balloon.c +0
-0@@ -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);
Quoted 4 lines
hw/virtio/virtio-balloon.c static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data) · line 266
Quoted 2 lines
hw/virtio/virtio-balloon.c +1
-1-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.
Quoted 2 lines
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.
Quoted 7 lines
We may use
assert(sizeof(struct virtio_balloon_config) == 8)somewhere, but to my taste it is a bit overkill. No?Thanks,
/mjt
Quoted 9 lines · Michael Tokarev
Quoted 8 lines · Luiz Capitulino
Use
sizeof(strucy virtio_balloon_config)instead.hw/virtio/virtio-balloon.c +0
-0@@ -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);
Quoted 7 lines
hw/virtio/virtio-balloon.c static void virtio_balloon_get_config(VirtIODevice *vdev, uint8_t *config_data) · line 266
Quoted 2 lines
hw/virtio/virtio-balloon.c +1
-1-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.
Quoted 3 lines
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.
Quoted 3 lines
Thanks,
/mjt
Signature · 1 line
MST
Michael Tokarev
AppliedQuoted 1 line · Luiz Capitulino
Use
sizeof(strucy virtio_balloon_config)instead.
Thanks, applied to the trivial patches queue (with the spelling fix).
/mjt
Quoted 4 lines · On Wed, 15 Jan 2014 20:10:31 +0400 Michael Tokarev <[email protected]> wrote:
Quoted 1 line · Luiz Capitulino
Use
sizeof(strucy virtio_balloon_config)instead.Thanks, applied to the trivial patches queue (with the spelling fix).
Thanks Michael.