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 <address@hidden>
> > Signed-off-by: Michael S. Tsirkin <address@hidden>
> > ---
> >  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);
> >  }
> >