| Message ID | 20150521115346.GJ23116@redhat.com |
|---|---|
| State | New |
| Headers | show |
Am 21.05.2015 um 13:53 schrieb Daniel P. Berrange: > On Thu, May 21, 2015 at 12:18:30PM +0100, Peter Maydell wrote: >> On 20 May 2015 at 16:51, Andreas Färber <afaerber@suse.de> wrote: >>> Hello Peter, >>> >>> This is my QOM (devices) patch queue. Please pull. >>> >>> Regards, >>> Andreas >>> >>> Cc: Peter Maydell <peter.maydell@linaro.org> >>> Cc: Eduardo Habkost <ehabkost@redhat.com> >>> Cc: Paolo Bonzini <pbonzini@redhat.com> >>> Cc: Daniel P. Berrange <berrange@redhat.com> >>> >>> The following changes since commit faa261a7fb254866bdd5b6a25ad94677945f21b4: >>> >>> Merge remote-tracking branch 'remotes/pmaydell/tags/pull-cocoa-20150519' into staging (2015-05-19 10:25:59 +0100) >>> >>> are available in the git repository at: >>> >>> git://github.com/afaerber/qemu-cpu.git tags/qom-devices-for-peter >>> >>> for you to fetch changes up to 28b86c32afbc53f9f06a5655da65f9d06fac1a3e: >>> >>> qom: Add object_property_add_const_link() (2015-05-20 17:40:47 +0200) >> >> Fails to build on my 32-bit ARM box, I'm afraid: >> >> tests/check-qom-proplist.c: In function 'test_dummy_badenum': >> tests/check-qom-proplist.c:225:6: error: value computed is not used >> [-Werror=unused-value] >> >> My guess is this is a gcc-version-dependent thing rather >> than particularly 32-bit or ARM related. It's using >> gcc (Ubuntu/Linaro 4.7.2-2ubuntu1) 4.7.2 > > Agreed, looks gcc version related not ARM/32-bit. > > I'm thinking the following change applied to the patch > "qom: add a object_property_add_enum helper method" will probably > fix it > > diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c > index 8b764a1..7400b1f 100644 > --- a/tests/check-qom-proplist.c > +++ b/tests/check-qom-proplist.c > @@ -222,7 +222,7 @@ static void test_dummy_badenum(void) > { > Error *err = NULL; > Object *parent = object_get_objects_root(); > - DUMMY_OBJECT( > + Object *dobj = > object_new_with_props(TYPE_DUMMY, > parent, > "dummy0", > @@ -230,8 +230,9 @@ static void test_dummy_badenum(void) > "bv", "yes", > "sv", "Hiss hiss hiss", > "av", "yeti", > - NULL)); > + NULL); > > + g_assert(dobj == NULL); > g_assert(err != NULL); > g_assert_cmpstr(error_get_pretty(err), ==, > "Invalid parameter 'yeti'"); Thanks, I've used this instead: - g_assert(err != NULL); + g_assert_null(dobj); + g_assert_nonnull(err); More places could be cleaned up that way... Regards, Andreas P.S. Quite obviously with gcc 4.8.3 there were no issues on my side.
On Wed, May 27, 2015 at 08:16:01PM +0200, Andreas Färber wrote: > Am 21.05.2015 um 13:53 schrieb Daniel P. Berrange: > > On Thu, May 21, 2015 at 12:18:30PM +0100, Peter Maydell wrote: > >> On 20 May 2015 at 16:51, Andreas Färber <afaerber@suse.de> wrote: > >>> Hello Peter, > >>> > >>> This is my QOM (devices) patch queue. Please pull. > >>> > >>> Regards, > >>> Andreas > >>> > >>> Cc: Peter Maydell <peter.maydell@linaro.org> > >>> Cc: Eduardo Habkost <ehabkost@redhat.com> > >>> Cc: Paolo Bonzini <pbonzini@redhat.com> > >>> Cc: Daniel P. Berrange <berrange@redhat.com> > >>> > >>> The following changes since commit faa261a7fb254866bdd5b6a25ad94677945f21b4: > >>> > >>> Merge remote-tracking branch 'remotes/pmaydell/tags/pull-cocoa-20150519' into staging (2015-05-19 10:25:59 +0100) > >>> > >>> are available in the git repository at: > >>> > >>> git://github.com/afaerber/qemu-cpu.git tags/qom-devices-for-peter > >>> > >>> for you to fetch changes up to 28b86c32afbc53f9f06a5655da65f9d06fac1a3e: > >>> > >>> qom: Add object_property_add_const_link() (2015-05-20 17:40:47 +0200) > >> > >> Fails to build on my 32-bit ARM box, I'm afraid: > >> > >> tests/check-qom-proplist.c: In function 'test_dummy_badenum': > >> tests/check-qom-proplist.c:225:6: error: value computed is not used > >> [-Werror=unused-value] > >> > >> My guess is this is a gcc-version-dependent thing rather > >> than particularly 32-bit or ARM related. It's using > >> gcc (Ubuntu/Linaro 4.7.2-2ubuntu1) 4.7.2 > > > > Agreed, looks gcc version related not ARM/32-bit. > > > > I'm thinking the following change applied to the patch > > "qom: add a object_property_add_enum helper method" will probably > > fix it > > > > diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c > > index 8b764a1..7400b1f 100644 > > --- a/tests/check-qom-proplist.c > > +++ b/tests/check-qom-proplist.c > > @@ -222,7 +222,7 @@ static void test_dummy_badenum(void) > > { > > Error *err = NULL; > > Object *parent = object_get_objects_root(); > > - DUMMY_OBJECT( > > + Object *dobj = > > object_new_with_props(TYPE_DUMMY, > > parent, > > "dummy0", > > @@ -230,8 +230,9 @@ static void test_dummy_badenum(void) > > "bv", "yes", > > "sv", "Hiss hiss hiss", > > "av", "yeti", > > - NULL)); > > + NULL); > > > > + g_assert(dobj == NULL); > > g_assert(err != NULL); > > g_assert_cmpstr(error_get_pretty(err), ==, > > "Invalid parameter 'yeti'"); > > Thanks, I've used this instead: > > - g_assert(err != NULL); > + g_assert_null(dobj); > + g_assert_nonnull(err); > > More places could be cleaned up that way... Since that caused failure with glib 2.22 could you revert that switch to g_assert_null/nonnull. Regards, Daniel
On 29 May 2015 at 14:51, Daniel P. Berrange <berrange@redhat.com> wrote: > Since that caused failure with glib 2.22 could you revert that switch > to g_assert_null/nonnull. BTW, David Gilbert is looking at whether we can use the glib support to make use of "newer than version X" APIs a compile error everywhere rather than just on boxes with the older glib, which should help cut down on this kind of problem in future. -- PMM
On Fri, May 29, 2015 at 02:57:12PM +0100, Peter Maydell wrote: > On 29 May 2015 at 14:51, Daniel P. Berrange <berrange@redhat.com> wrote: > > Since that caused failure with glib 2.22 could you revert that switch > > to g_assert_null/nonnull. > > BTW, David Gilbert is looking at whether we can use the glib support > to make use of "newer than version X" APIs a compile error everywhere > rather than just on boxes with the older glib, which should help > cut down on this kind of problem in future. It seem these g_assert_nonnull/null functions are in fact just trivial macros, so we could alternatively add them to our glib-compat.h file #define g_assert_true(expr) G_STMT_START { \ if G_LIKELY (expr) ; else \ g_assertion_message (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ "'" #expr "' should be TRUE"); \ } G_STMT_END #define g_assert_false(expr) G_STMT_START { \ if G_LIKELY (!(expr)) ; else \ g_assertion_message (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ "'" #expr "' should be FALSE"); \ } G_STMT_END #define g_assert_null(expr) G_STMT_START { if G_LIKELY ((expr) == NULL) ; else \ g_assertion_message (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ "'" #expr "' should be NULL"); \ } G_STMT_END #define g_assert_nonnull(expr) G_STMT_START { \ if G_LIKELY ((expr) != NULL) ; else \ g_assertion_message (G_LOG_DOMAIN, __FILE__, __LINE__, G_STRFUNC, \ "'" #expr "' should not be NULL"); \ Regards, Daniel
[apologies for getting David G's email address wrong first time round] On 29 May 2015 at 15:08, Daniel P. Berrange <berrange@redhat.com> wrote: > On Fri, May 29, 2015 at 02:57:12PM +0100, Peter Maydell wrote: >> On 29 May 2015 at 14:51, Daniel P. Berrange <berrange@redhat.com> wrote: >> > Since that caused failure with glib 2.22 could you revert that switch >> > to g_assert_null/nonnull. >> >> BTW, David Gilbert is looking at whether we can use the glib support >> to make use of "newer than version X" APIs a compile error everywhere >> rather than just on boxes with the older glib, which should help >> cut down on this kind of problem in future. > > It seem these g_assert_nonnull/null functions are in fact just trivial > macros, so we could alternatively add them to our glib-compat.h file Well, we could, but I don't really see the point compared to just using g_assert()... -- PMM
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c index 8b764a1..7400b1f 100644 --- a/tests/check-qom-proplist.c +++ b/tests/check-qom-proplist.c @@ -222,7 +222,7 @@ static void test_dummy_badenum(void) { Error *err = NULL; Object *parent = object_get_objects_root(); - DUMMY_OBJECT( + Object *dobj = object_new_with_props(TYPE_DUMMY, parent, "dummy0", @@ -230,8 +230,9 @@ static void test_dummy_badenum(void) "bv", "yes", "sv", "Hiss hiss hiss", "av", "yeti", - NULL)); + NULL); + g_assert(dobj == NULL); g_assert(err != NULL); g_assert_cmpstr(error_get_pretty(err), ==, "Invalid parameter 'yeti'");