[Qemu-devel] [PATCH for-1.7 1/2] acpi-build: fix build on glib < 2.22

[Qemu-devel] [PATCH for-1.7 1/2] acpi-build: fix build on glib < 2.22

[Michael S. Tsirkin]

g_string_vprintf was only introduced in 2.24 so switch to vsnprintf
instead.  A bit uglier but name size is fixed at 4 bytes here so it's
easy.

Reported-by: Richard Henderson <rth@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/acpi-build.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 486e705..59a17df 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -287,16 +287,17 @@ static inline void build_append_array(GArray *array, GArray *val)
 
 static void build_append_nameseg(GArray *array, const char *format, ...)
 {
-    GString *s = g_string_new("");
+    /* It would be nicer to use g_string_vprintf but it's only there in 2.22 */
+    char s[] = "XXXX";
+    int len;
     va_list args;
 
     va_start(args, format);
-    g_string_vprintf(s, format, args);
+    len = vsnprintf(s, sizeof s, format, args);
     va_end(args);
 
-    assert(s->len == 4);
-    g_array_append_vals(array, s->str, s->len);
-    g_string_free(s, true);
+    assert(len == 4);
+    g_array_append_vals(array, s, len);
 }
 
 /* 5.4 Definition Block Encoding */
-- 
MST

[Qemu-devel] [PATCH for-1.7 2/2] acpi-build: fix build on glib < 2.14

[Michael S. Tsirkin]

g_array_get_element_size was only added in glib 2.14.
Fortunately we don't use it for any arrays where
element size is > 1, so just add an assert.

Reported-by: Richard Henderson <rth@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/i386/acpi-build.c         | 5 ++++-
 hw/i386/bios-linker-loader.c | 8 ++++----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 59a17df..5f36e7e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -425,7 +425,10 @@ static inline void *acpi_data_push(GArray *table_data, unsigned size)
 
 static unsigned acpi_data_len(GArray *table)
 {
-    return table->len * g_array_get_element_size(table);
+#if GLIB_CHECK_VERSION(2, 14, 0)
+    assert(g_array_get_element_size(table) == 1);
+#endif
+    return table->len;
 }
 
 static void acpi_align_size(GArray *blob, unsigned align)
diff --git a/hw/i386/bios-linker-loader.c b/hw/i386/bios-linker-loader.c
index 0833853..fd23611 100644
--- a/hw/i386/bios-linker-loader.c
+++ b/hw/i386/bios-linker-loader.c
@@ -90,7 +90,7 @@ enum {
 
 GArray *bios_linker_loader_init(void)
 {
-    return g_array_new(false, true /* clear */, sizeof(BiosLinkerLoaderEntry));
+    return g_array_new(false, true /* clear */, 1);
 }
 
 /* Free linker wrapper and return the linker array. */
@@ -115,7 +115,7 @@ void bios_linker_loader_alloc(GArray *linker,
                                     BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH);
 
     /* Alloc entries must come first, so prepend them */
-    g_array_prepend_val(linker, entry);
+    g_array_prepend_vals(linker, &entry, sizeof entry);
 }
 
 void bios_linker_loader_add_checksum(GArray *linker, const char *file,
@@ -132,7 +132,7 @@ void bios_linker_loader_add_checksum(GArray *linker, const char *file,
     entry.cksum.start = cpu_to_le32((uint8_t *)start - (uint8_t *)table);
     entry.cksum.length = cpu_to_le32(size);
 
-    g_array_append_val(linker, entry);
+    g_array_append_vals(linker, &entry, sizeof entry);
 }
 
 void bios_linker_loader_add_pointer(GArray *linker,
@@ -154,5 +154,5 @@ void bios_linker_loader_add_pointer(GArray *linker,
     assert(pointer_size == 1 || pointer_size == 2 ||
            pointer_size == 4 || pointer_size == 8);
 
-    g_array_append_val(linker, entry);
+    g_array_append_vals(linker, &entry, sizeof entry);
 }
-- 
MST

Re: [Qemu-devel] [PATCH for-1.7 1/2] acpi-build: fix build on glib < 2.22

[Paolo Bonzini]

Il 21/11/2013 13:17, Michael S. Tsirkin ha scritto:
> g_string_vprintf was only introduced in 2.24 so switch to vsnprintf
> instead.  A bit uglier but name size is fixed at 4 bytes here so it's
> easy.
> 
> Reported-by: Richard Henderson <rth@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/i386/acpi-build.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 486e705..59a17df 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -287,16 +287,17 @@ static inline void build_append_array(GArray *array, GArray *val)
>  
>  static void build_append_nameseg(GArray *array, const char *format, ...)
>  {
> -    GString *s = g_string_new("");
> +    /* It would be nicer to use g_string_vprintf but it's only there in 2.22 */
> +    char s[] = "XXXX";
> +    int len;
>      va_list args;
>  
>      va_start(args, format);
> -    g_string_vprintf(s, format, args);
> +    len = vsnprintf(s, sizeof s, format, args);
>      va_end(args);
>  
> -    assert(s->len == 4);
> -    g_array_append_vals(array, s->str, s->len);
> -    g_string_free(s, true);
> +    assert(len == 4);
> +    g_array_append_vals(array, s, len);
>  }
>  
>  /* 5.4 Definition Block Encoding */
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH for-1.7 2/2] acpi-build: fix build on glib < 2.14

[Paolo Bonzini]

Il 21/11/2013 13:17, Michael S. Tsirkin ha scritto:
> g_array_get_element_size was only added in glib 2.14.
> Fortunately we don't use it for any arrays where
> element size is > 1, so just add an assert.
> 
> Reported-by: Richard Henderson <rth@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/i386/acpi-build.c         | 5 ++++-
>  hw/i386/bios-linker-loader.c | 8 ++++----
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 59a17df..5f36e7e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -425,7 +425,10 @@ static inline void *acpi_data_push(GArray *table_data, unsigned size)
>  
>  static unsigned acpi_data_len(GArray *table)
>  {
> -    return table->len * g_array_get_element_size(table);
> +#if GLIB_CHECK_VERSION(2, 14, 0)
> +    assert(g_array_get_element_size(table) == 1);
> +#endif
> +    return table->len;
>  }
>  
>  static void acpi_align_size(GArray *blob, unsigned align)

This looks good, but is the below part necessary?  It's ugly!

Paolo

> diff --git a/hw/i386/bios-linker-loader.c b/hw/i386/bios-linker-loader.c
> index 0833853..fd23611 100644
> --- a/hw/i386/bios-linker-loader.c
> +++ b/hw/i386/bios-linker-loader.c
> @@ -90,7 +90,7 @@ enum {
>  
>  GArray *bios_linker_loader_init(void)
>  {
> -    return g_array_new(false, true /* clear */, sizeof(BiosLinkerLoaderEntry));
> +    return g_array_new(false, true /* clear */, 1);
>  }
>  
>  /* Free linker wrapper and return the linker array. */
> @@ -115,7 +115,7 @@ void bios_linker_loader_alloc(GArray *linker,
>                                      BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH);
>  
>      /* Alloc entries must come first, so prepend them */
> -    g_array_prepend_val(linker, entry);
> +    g_array_prepend_vals(linker, &entry, sizeof entry);
>  }
>  
>  void bios_linker_loader_add_checksum(GArray *linker, const char *file,
> @@ -132,7 +132,7 @@ void bios_linker_loader_add_checksum(GArray *linker, const char *file,
>      entry.cksum.start = cpu_to_le32((uint8_t *)start - (uint8_t *)table);
>      entry.cksum.length = cpu_to_le32(size);
>  
> -    g_array_append_val(linker, entry);
> +    g_array_append_vals(linker, &entry, sizeof entry);
>  }
>  
>  void bios_linker_loader_add_pointer(GArray *linker,
> @@ -154,5 +154,5 @@ void bios_linker_loader_add_pointer(GArray *linker,
>      assert(pointer_size == 1 || pointer_size == 2 ||
>             pointer_size == 4 || pointer_size == 8);
>  
> -    g_array_append_val(linker, entry);
> +    g_array_append_vals(linker, &entry, sizeof entry);
>  }
> 

Re: [Qemu-devel] [PATCH for-1.7 2/2] acpi-build: fix build on glib < 2.14

[Michael S. Tsirkin]

On Thu, Nov 21, 2013 at 02:54:17PM +0100, Paolo Bonzini wrote:
> Il 21/11/2013 13:17, Michael S. Tsirkin ha scritto:
> > g_array_get_element_size was only added in glib 2.14.
> > Fortunately we don't use it for any arrays where
> > element size is > 1, so just add an assert.
> > 
> > Reported-by: Richard Henderson <rth@redhat.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  hw/i386/acpi-build.c         | 5 ++++-
> >  hw/i386/bios-linker-loader.c | 8 ++++----
> >  2 files changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 59a17df..5f36e7e 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -425,7 +425,10 @@ static inline void *acpi_data_push(GArray *table_data, unsigned size)
> >  
> >  static unsigned acpi_data_len(GArray *table)
> >  {
> > -    return table->len * g_array_get_element_size(table);
> > +#if GLIB_CHECK_VERSION(2, 14, 0)
> > +    assert(g_array_get_element_size(table) == 1);
> > +#endif
> > +    return table->len;
> >  }
> >  
> >  static void acpi_align_size(GArray *blob, unsigned align)
> 
> This looks good, but is the below part necessary?  It's ugly!
> 
> Paolo

Unfortunately yes. With old glib we don't know element size
so we have to make all elements same size (1 byte).

Without making change below change above will trigger assertions.

> > diff --git a/hw/i386/bios-linker-loader.c b/hw/i386/bios-linker-loader.c
> > index 0833853..fd23611 100644
> > --- a/hw/i386/bios-linker-loader.c
> > +++ b/hw/i386/bios-linker-loader.c
> > @@ -90,7 +90,7 @@ enum {
> >  
> >  GArray *bios_linker_loader_init(void)
> >  {
> > -    return g_array_new(false, true /* clear */, sizeof(BiosLinkerLoaderEntry));
> > +    return g_array_new(false, true /* clear */, 1);
> >  }
> >  
> >  /* Free linker wrapper and return the linker array. */
> > @@ -115,7 +115,7 @@ void bios_linker_loader_alloc(GArray *linker,
> >                                      BIOS_LINKER_LOADER_ALLOC_ZONE_HIGH);
> >  
> >      /* Alloc entries must come first, so prepend them */
> > -    g_array_prepend_val(linker, entry);
> > +    g_array_prepend_vals(linker, &entry, sizeof entry);
> >  }
> >  
> >  void bios_linker_loader_add_checksum(GArray *linker, const char *file,
> > @@ -132,7 +132,7 @@ void bios_linker_loader_add_checksum(GArray *linker, const char *file,
> >      entry.cksum.start = cpu_to_le32((uint8_t *)start - (uint8_t *)table);
> >      entry.cksum.length = cpu_to_le32(size);
> >  
> > -    g_array_append_val(linker, entry);
> > +    g_array_append_vals(linker, &entry, sizeof entry);
> >  }
> >  
> >  void bios_linker_loader_add_pointer(GArray *linker,
> > @@ -154,5 +154,5 @@ void bios_linker_loader_add_pointer(GArray *linker,
> >      assert(pointer_size == 1 || pointer_size == 2 ||
> >             pointer_size == 4 || pointer_size == 8);
> >  
> > -    g_array_append_val(linker, entry);
> > +    g_array_append_vals(linker, &entry, sizeof entry);
> >  }
> > 

Re: [Qemu-devel] [PATCH for-1.7 2/2] acpi-build: fix build on glib < 2.14

[Paolo Bonzini]

Il 21/11/2013 15:16, Michael S. Tsirkin ha scritto:
> > This looks good, but is the below part necessary?  It's ugly!
> 
> Unfortunately yes. With old glib we don't know element size
> so we have to make all elements same size (1 byte).
> 
> Without making change below change above will trigger assertions.

Ok, then please mention it in the commit message.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo