Re: [Qemu-devel] [PATCH] tests/libqos: Utilize newer glib spawn check

[Thomas Huth]

On 2018-08-21 21:05, Eric Blake wrote:
> During development, I got a 'make check' failure that claimed:
> 
> qemu-img returned status code 32512
> **
> ERROR:tests/libqos/libqos.c:202:mkimg: assertion failed: (!rc)
> 
> But 32512 is too big for a normal exit status value, which means we
> failed to use WEXITSTATUS() to shift the bits to the desired value
> for printing.  However, instead of worrying about how to portably
> parse g_spawn()'s rc in the proper platform-dependent manner, it's
> better to just rely on the fact that we now require glib 2.40 (since
> commit e7b3af815) and can therefore use glib's portable checker
> instead, where the message under my same condition improves to:
> 
> Child process exited with code 127
> **
> ERROR:tests/libqos/libqos.c:192:mkimg: assertion failed: (ret && !err)
> 
> Signed-off-by: Eric Blake <address@hidden>
> 
> ---
> This appears to be the only remaining vestige of a comment mentioning
> glib < 2.40.
> ---
>  tests/libqos/libqos.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)
> 
> diff --git a/tests/libqos/libqos.c b/tests/libqos/libqos.c
> index 013ca68581c..c5141873448 100644
> --- a/tests/libqos/libqos.c
> +++ b/tests/libqos/libqos.c
> @@ -185,22 +185,12 @@ void mkimg(const char *file, const char *fmt, unsigned 
> size_mb)
>      cli = g_strdup_printf("%s create -f %s %s %uM", qemu_img_abs_path,
>                            fmt, file, size_mb);
>      ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err);
> -    if (err) {
> +    if (err || !g_spawn_check_exit_status(rc, &err)) {
>          fprintf(stderr, "%s\n", err->message);
>          g_error_free(err);
>      }
>      g_assert(ret && !err);
> 
> -    /* In glib 2.34, we have g_spawn_check_exit_status. in 2.12, we don't.
> -     * glib 2.43.91 implementation assumes that any non-zero is an error for
> -     * windows, but uses extra precautions for Linux. However,
> -     * 0 is only possible if the program exited normally, so that should be
> -     * sufficient for our purposes on all platforms, here. */
> -    if (rc) {
> -        fprintf(stderr, "qemu-img returned status code %d\n", rc);
> -    }
> -    g_assert(!rc);
> -
>      g_free(out);
>      g_free(out2);
>      g_free(cli);
> 

Reviewed-by: Thomas Huth <address@hidden>