On 04/06/2017 10:49 AM, Eric Blake wrote: > On 04/06/2017 10:41 AM, Dr. David Alan Gilbert (git) wrote: >> From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> >> >> Gcc 7 (on Fedora 26) spotted odd use of integers instead of a >> boolean; it's got a point. >> >> Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com> >> --- >> tests/check-qdict.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/tests/check-qdict.c b/tests/check-qdict.c >> index 81162ee572..6f3fbcf9c1 100644 >> --- a/tests/check-qdict.c >> +++ b/tests/check-qdict.c >> @@ -559,7 +559,7 @@ static void qdict_join_test(void) >> g_assert(qdict_size(dict1) == 2); >> g_assert(qdict_size(dict2) == !overwrite); >> >> - g_assert(qdict_get_int(dict1, "foo") == overwrite ? 84 : 42); >> + g_assert(qdict_get_int(dict1, "foo") == (overwrite ? 84 : 42)); > > How is the test passing pre-patch, and why does it not change the test > post-patch? Does that mean that overwrite is not doing what we expected?
Replying to myself: Pre-patch, it was reached twice (once for overwrite=false, once for overwrite=true), as: (42 == false) ? 84 : 42 (84 == true) ? 84 : 42 which simplifies to 42 on both iterations, and g_assert(42) succeeds. In fact, this is a tautology - no matter WHAT value we encounter, the assert succeeds, so we are not actually testing that the value matters. Post-patch, it becomes: 42 == (false ? 84 : 42) 84 == (true ? 84 : 42) which is then asserting that we actually have the value we expect. Reviewed-by: Eric Blake <ebl...@redhat.com> Safe for 2.9, but late enough that it also doesn't matter if it slips until 2.10. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature