Re: [Qemu-devel] [PATCH 5/5] migration: Route more error paths

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]