* [Qemu-devel] [PATCH for-1.7 1/2] acpi-build: fix build on glib < 2.22 @ 2013-11-21 12:17 Michael S. Tsirkin 2013-11-21 12:17 ` [Qemu-devel] [PATCH for-1.7 2/2] acpi-build: fix build on glib < 2.14 Michael S. Tsirkin 2013-11-21 13:51 ` [Qemu-devel] [PATCH for-1.7 1/2] acpi-build: fix build on glib < 2.22 Paolo Bonzini 0 siblings, 2 replies; 6+ messages in thread From: Michael S. Tsirkin @ 2013-11-21 12:17 UTC (permalink / raw) To: qemu-devel; +Cc: Richard Henderson, Anthony Liguori 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH for-1.7 2/2] acpi-build: fix build on glib < 2.14 2013-11-21 12:17 [Qemu-devel] [PATCH for-1.7 1/2] acpi-build: fix build on glib < 2.22 Michael S. Tsirkin @ 2013-11-21 12:17 ` Michael S. Tsirkin 2013-11-21 13:54 ` Paolo Bonzini 2013-11-21 13:51 ` [Qemu-devel] [PATCH for-1.7 1/2] acpi-build: fix build on glib < 2.22 Paolo Bonzini 1 sibling, 1 reply; 6+ messages in thread From: Michael S. Tsirkin @ 2013-11-21 12:17 UTC (permalink / raw) To: qemu-devel; +Cc: Richard Henderson, Anthony Liguori 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH for-1.7 2/2] acpi-build: fix build on glib < 2.14 2013-11-21 12:17 ` [Qemu-devel] [PATCH for-1.7 2/2] acpi-build: fix build on glib < 2.14 Michael S. Tsirkin @ 2013-11-21 13:54 ` Paolo Bonzini 2013-11-21 14:16 ` Michael S. Tsirkin 0 siblings, 1 reply; 6+ messages in thread From: Paolo Bonzini @ 2013-11-21 13:54 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: qemu-devel, Anthony Liguori, Richard Henderson 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); > } > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH for-1.7 2/2] acpi-build: fix build on glib < 2.14 2013-11-21 13:54 ` Paolo Bonzini @ 2013-11-21 14:16 ` Michael S. Tsirkin 2013-11-21 14:16 ` Paolo Bonzini 0 siblings, 1 reply; 6+ messages in thread From: Michael S. Tsirkin @ 2013-11-21 14:16 UTC (permalink / raw) To: Paolo Bonzini; +Cc: qemu-devel, Anthony Liguori, Richard Henderson 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); > > } > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH for-1.7 2/2] acpi-build: fix build on glib < 2.14 2013-11-21 14:16 ` Michael S. Tsirkin @ 2013-11-21 14:16 ` Paolo Bonzini 0 siblings, 0 replies; 6+ messages in thread From: Paolo Bonzini @ 2013-11-21 14:16 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: qemu-devel, Anthony Liguori, Richard Henderson 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH for-1.7 1/2] acpi-build: fix build on glib < 2.22 2013-11-21 12:17 [Qemu-devel] [PATCH for-1.7 1/2] acpi-build: fix build on glib < 2.22 Michael S. Tsirkin 2013-11-21 12:17 ` [Qemu-devel] [PATCH for-1.7 2/2] acpi-build: fix build on glib < 2.14 Michael S. Tsirkin @ 2013-11-21 13:51 ` Paolo Bonzini 1 sibling, 0 replies; 6+ messages in thread From: Paolo Bonzini @ 2013-11-21 13:51 UTC (permalink / raw) To: Michael S. Tsirkin; +Cc: qemu-devel, Anthony Liguori, Richard Henderson 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> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-11-21 14:17 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-21 12:17 [Qemu-devel] [PATCH for-1.7 1/2] acpi-build: fix build on glib < 2.22 Michael S. Tsirkin 2013-11-21 12:17 ` [Qemu-devel] [PATCH for-1.7 2/2] acpi-build: fix build on glib < 2.14 Michael S. Tsirkin 2013-11-21 13:54 ` Paolo Bonzini 2013-11-21 14:16 ` Michael S. Tsirkin 2013-11-21 14:16 ` Paolo Bonzini 2013-11-21 13:51 ` [Qemu-devel] [PATCH for-1.7 1/2] acpi-build: fix build on glib < 2.22 Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).