On 09/20/2017 09:20 AM, Dr. David Alan Gilbert wrote: >>> /* Save file with vmstate */ >>> - vmstate_save_state(f, desc, obj, NULL); >>> + g_assert(!vmstate_save_state(f, desc, obj, NULL)); >> >> Though this is test code, isn't putting anything with a side effect into an >> assert expression a very bad pattern in general? > > Hmm; ok I've changed this but I'm not really convinced; the whole point > of an asser in a test is to actually run it, and I think the g_assert > prints the text that failed, so it gives you a much better error inline.
glib doesn't make life any easier by having two different flavors of assertions. g_assert() can be disabled at compile-time (side effects are lost). It is roughly akin to assert(); and the output when an assertion fails isn't that much more useful than what you get with plain assert(). g_assert_cmpint() cannot be disabled at compile-time, but can have a runtime switch on whether it abort()s or continues on. Here, the output when something fails is MUCH more pleasant to read. But the fact that there are two separate families of g_assert* behaviors just makes it easier to live by the mantra that side-effects don't belong inside assertions. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
