| Message ID | 20180821190516.264411-1-eblake@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | tests/libqos: Utilize newer glib spawn check | expand |
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 <eblake@redhat.com> > > --- > 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 <thuth@redhat.com>
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);
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 <eblake@redhat.com> --- 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(-)