Patchwork virtio-serial: Simplify virtio_serial_load()

login
register
mail settings
Submitter Markus Armbruster
Date May 31, 2010, 2:19 p.m.
Message ID <m3ocfwp30g.fsf@blackfin.pond.sub.org>
Download mbox | patch
Permalink /patch/54099/
State New
Headers show

Comments

Markus Armbruster - May 31, 2010, 2:19 p.m.
For all i, ports_map[i] is used in and only in the i-th iteration.
Replace the dynamic array by a scalar variable.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/virtio-serial-bus.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)
Amit Shah - June 1, 2010, 8:22 a.m.
On (Mon) May 31 2010 [16:19:43], Markus Armbruster wrote:
> For all i, ports_map[i] is used in and only in the i-th iteration.
> Replace the dynamic array by a scalar variable.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/virtio-serial-bus.c |   12 +++---------
>  1 files changed, 3 insertions(+), 9 deletions(-)

Thanks,

Acked-by: Amit Shah <amit.shah@redhat.com>

		Amit
Alon Levy - June 1, 2010, 1:23 p.m.
----- "Markus Armbruster" <armbru@redhat.com> wrote:

> For all i, ports_map[i] is used in and only in the i-th iteration.
> Replace the dynamic array by a scalar variable.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/virtio-serial-bus.c |   12 +++---------
>  1 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index 3ce95e8..bcc6d5d 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -492,8 +492,7 @@ static int virtio_serial_load(QEMUFile *f, void
> *opaque, int version_id)
>  {
>      VirtIOSerial *s = opaque;
>      VirtIOSerialPort *port;
> -    size_t ports_map_size;
> -    uint32_t max_nr_ports, nr_active_ports, *ports_map;
> +    uint32_t max_nr_ports, nr_active_ports, ports_map;
>      unsigned int i;
>  
>      if (version_id > 2) {
> @@ -517,22 +516,17 @@ static int virtio_serial_load(QEMUFile *f, void
> *opaque, int version_id)
>          return -EINVAL;
>      }
>  
> -    ports_map_size = sizeof(uint32_t) * (max_nr_ports + 31) / 32;
> -    ports_map = qemu_malloc(ports_map_size);
> -
>      for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
> -        qemu_get_be32s(f, &ports_map[i]);
> +        qemu_get_be32s(f, &ports_map);
>  
> -        if (ports_map[i] != s->ports_map[i]) {
> +        if (ports_map != s->ports_map[i]) {
>              /*
>               * Ports active on source and destination don't
>               * match. Fail migration.
>               */
> -            qemu_free(ports_map);
>              return -EINVAL;
>          }
>      }
> -    qemu_free(ports_map);
>  
>      qemu_get_be32s(f, &nr_active_ports);
>  
> -- 
> 1.6.6.1

ACK
Markus Armbruster - June 26, 2010, 5:54 a.m.
Did this fall through the cracks?

Markus Armbruster <armbru@redhat.com> writes:

> For all i, ports_map[i] is used in and only in the i-th iteration.
> Replace the dynamic array by a scalar variable.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/virtio-serial-bus.c |   12 +++---------
>  1 files changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index 3ce95e8..bcc6d5d 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -492,8 +492,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
>  {
>      VirtIOSerial *s = opaque;
>      VirtIOSerialPort *port;
> -    size_t ports_map_size;
> -    uint32_t max_nr_ports, nr_active_ports, *ports_map;
> +    uint32_t max_nr_ports, nr_active_ports, ports_map;
>      unsigned int i;
>  
>      if (version_id > 2) {
> @@ -517,22 +516,17 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
>          return -EINVAL;
>      }
>  
> -    ports_map_size = sizeof(uint32_t) * (max_nr_ports + 31) / 32;
> -    ports_map = qemu_malloc(ports_map_size);
> -
>      for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
> -        qemu_get_be32s(f, &ports_map[i]);
> +        qemu_get_be32s(f, &ports_map);
>  
> -        if (ports_map[i] != s->ports_map[i]) {
> +        if (ports_map != s->ports_map[i]) {
>              /*
>               * Ports active on source and destination don't
>               * match. Fail migration.
>               */
> -            qemu_free(ports_map);
>              return -EINVAL;
>          }
>      }
> -    qemu_free(ports_map);
>  
>      qemu_get_be32s(f, &nr_active_ports);
Aurelien Jarno - June 30, 2010, 6:39 p.m.
On Mon, May 31, 2010 at 04:19:43PM +0200, Markus Armbruster wrote:
> For all i, ports_map[i] is used in and only in the i-th iteration.
> Replace the dynamic array by a scalar variable.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/virtio-serial-bus.c |   12 +++---------
>  1 files changed, 3 insertions(+), 9 deletions(-)

Thanks, applied.

> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index 3ce95e8..bcc6d5d 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -492,8 +492,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
>  {
>      VirtIOSerial *s = opaque;
>      VirtIOSerialPort *port;
> -    size_t ports_map_size;
> -    uint32_t max_nr_ports, nr_active_ports, *ports_map;
> +    uint32_t max_nr_ports, nr_active_ports, ports_map;
>      unsigned int i;
>  
>      if (version_id > 2) {
> @@ -517,22 +516,17 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
>          return -EINVAL;
>      }
>  
> -    ports_map_size = sizeof(uint32_t) * (max_nr_ports + 31) / 32;
> -    ports_map = qemu_malloc(ports_map_size);
> -
>      for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
> -        qemu_get_be32s(f, &ports_map[i]);
> +        qemu_get_be32s(f, &ports_map);
>  
> -        if (ports_map[i] != s->ports_map[i]) {
> +        if (ports_map != s->ports_map[i]) {
>              /*
>               * Ports active on source and destination don't
>               * match. Fail migration.
>               */
> -            qemu_free(ports_map);
>              return -EINVAL;
>          }
>      }
> -    qemu_free(ports_map);
>  
>      qemu_get_be32s(f, &nr_active_ports);
>  
> -- 
> 1.6.6.1
> 
> 
>

Patch

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 3ce95e8..bcc6d5d 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -492,8 +492,7 @@  static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
 {
     VirtIOSerial *s = opaque;
     VirtIOSerialPort *port;
-    size_t ports_map_size;
-    uint32_t max_nr_ports, nr_active_ports, *ports_map;
+    uint32_t max_nr_ports, nr_active_ports, ports_map;
     unsigned int i;
 
     if (version_id > 2) {
@@ -517,22 +516,17 @@  static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
         return -EINVAL;
     }
 
-    ports_map_size = sizeof(uint32_t) * (max_nr_ports + 31) / 32;
-    ports_map = qemu_malloc(ports_map_size);
-
     for (i = 0; i < (max_nr_ports + 31) / 32; i++) {
-        qemu_get_be32s(f, &ports_map[i]);
+        qemu_get_be32s(f, &ports_map);
 
-        if (ports_map[i] != s->ports_map[i]) {
+        if (ports_map != s->ports_map[i]) {
             /*
              * Ports active on source and destination don't
              * match. Fail migration.
              */
-            qemu_free(ports_map);
             return -EINVAL;
         }
     }
-    qemu_free(ports_map);
 
     qemu_get_be32s(f, &nr_active_ports);