Re: [Qemu-devel] [PATCH] glib-compat: work around g_test_message bug with subprocess tests

[Eric Blake <eblake@redhat.com>]

On 11/27/18 12:35 PM, Paolo Bonzini wrote:
> Subprocesses are created by glib without leaving the file descriptors
> open.  Therefore, g_test_message (and assertion failures, but those
> trigger when things are going bad anyway) will think that it is writing
> to the log file descriptor, but while actually stomping on the QMP
> file descriptor or similar.  This causes spurious failures, which are
> as nice to debug as the reader can imagine.  While I have opened a
> pull request on GLib, this will probably take a while to propagate
> to distros.
> 
> I found this while working on qgraph, but the fix is generic.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   include/glib-compat.h | 6 ++++++
>   1 file changed, 6 insertions(+)

Wow. I don't envy the debug session that you went through to finally 
realize this problem.

Reviewed-by: Eric Blake <eblake@redhat.com>

I think this is safe for 3.1-rc3 if you want it there, as minimizing 
spurious test failures is a good thing.

> diff --git a/include/glib-compat.h b/include/glib-compat.h
> index fdf95a2..989b9ef 100644
> --- a/include/glib-compat.h
> +++ b/include/glib-compat.h
> @@ -113,4 +113,10 @@ gint g_poll_fixed(GPollFD *fds, guint nfds, gint timeout);
>   
>   #pragma GCC diagnostic pop
>   
> +/* See https://gitlab.gnome.org/GNOME/glib/merge_requests/501 */
> +#define g_test_message(...)							\
> +	do {									\
> +		if (!g_test_subprocess()) g_test_message(__VA_ARGS__);		\

I'm surprised checkpatch.pl doesn't complain about missing {} in this 
macro expansion, but doing it right would mean 2 more lines, so I'm fine 
overlooking the style.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org