diff mbox

[PULL,for-1.7,v2,4/6] acpi-build: fix build on glib < 2.14

Message ID 1385379990-32093-5-git-send-email-mst@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin Nov. 25, 2013, 11:48 a.m. UTC 
g_array_get_element_size was only added in glib 2.14,
there's no way to find element size in with an older glib.

Fortunately we only use a single table (linker) where element size > 1.
Switch element size to 1 everywhere, then we can just look at len field
to get table size in bytes.

Add an assert to make sure we catch any violations of this rule.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
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(-)

Comments

Erik Rull Nov. 25, 2013, 8:14 p.m. UTC | #1 
Michael S. Tsirkin wrote:
> g_array_get_element_size was only added in glib 2.14,
> there's no way to find element size in with an older glib.
>
> Fortunately we only use a single table (linker) where element size > 1.
> Switch element size to 1 everywhere, then we can just look at len field
> to get table size in bytes.
>
> Add an assert to make sure we catch any violations of this rule.
>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> 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);
>   }
>

Hi all,

acpi-build.c has another occurence of g_array_get_element_size in 
acpi_align_size. Please put another ifdef for the older glib here, too.

Thanks.

Best regards,

Erik
Richard Henderson Nov. 25, 2013, 8:26 p.m. UTC | #2 
On 11/25/2013 09:48 PM, Michael S. Tsirkin wrote:
> +#if GLIB_CHECK_VERSION(2, 14, 0)
> +    assert(g_array_get_element_size(table) == 1);
> +#endif

https://developer.gnome.org/glib/2.28/glib-Arrays.html#g-array-get-element-size

says "Since 2.22", not 2.14.


r~
Michael S. Tsirkin Nov. 25, 2013, 8:59 p.m. UTC | #3 
On Mon, Nov 25, 2013 at 09:14:19PM +0100, Erik Rull wrote:
> Michael S. Tsirkin wrote:
> >g_array_get_element_size was only added in glib 2.14,
> >there's no way to find element size in with an older glib.
> >
> >Fortunately we only use a single table (linker) where element size > 1.
> >Switch element size to 1 everywhere, then we can just look at len field
> >to get table size in bytes.
> >
> >Add an assert to make sure we catch any violations of this rule.
> >
> >Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> >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);
> >  }
> >
> 
> Hi all,
> 
> acpi-build.c has another occurence of g_array_get_element_size in
> acpi_align_size. Please put another ifdef for the older glib here,
> too.
> 
> Thanks.
> 
> Best regards,
> 
> Erik
> 

Do you have a system where this fails then?
diff mbox

Patch

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);
 }