| Message ID | 20190123120759.7162-2-jusual@mail.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests/microbit-test: Add UART device test | expand |
On 2019-01-23 13:07, Julia Suvorova wrote: > Run qtest with a socket that connects QEMU chardev and test code. > > Signed-off-by: Julia Suvorova <jusual@mail.ru> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > tests/libqtest.c | 25 +++++++++++++++++++++++++ > tests/libqtest.h | 11 +++++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/tests/libqtest.c b/tests/libqtest.c > index 55750dd68d..6fb30855fa 100644 > --- a/tests/libqtest.c > +++ b/tests/libqtest.c > @@ -315,6 +315,31 @@ QTestState *qtest_initf(const char *fmt, ...) > return s; > } > > +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd) > +{ > + int sock_fd_init; > + char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX"; > + QTestState *qts; > + > + g_assert_true(mkdtemp(sock_dir) != NULL); Oh, nice, I wasn't aware of g_assert_true yet, that's a good idea here, indeed. Reviewed-by: Thomas Huth <thuth@redhat.com>
Julia Suvorova <jusual@mail.ru> writes: > Run qtest with a socket that connects QEMU chardev and test code. > > Signed-off-by: Julia Suvorova <jusual@mail.ru> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > --- > tests/libqtest.c | 25 +++++++++++++++++++++++++ > tests/libqtest.h | 11 +++++++++++ > 2 files changed, 36 insertions(+) > > diff --git a/tests/libqtest.c b/tests/libqtest.c > index 55750dd68d..6fb30855fa 100644 > --- a/tests/libqtest.c > +++ b/tests/libqtest.c > @@ -315,6 +315,31 @@ QTestState *qtest_initf(const char *fmt, ...) > return s; > } > > +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd) > +{ > + int sock_fd_init; > + char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX"; > + QTestState *qts; > + > + g_assert_true(mkdtemp(sock_dir) != NULL); > + sock_path = g_strdup_printf("%s/sock", sock_dir); > + > + sock_fd_init = init_socket(sock_path); > + > + qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s", > + sock_path, extra_args); > + > + *sock_fd = socket_accept(sock_fd_init); > + > + unlink(sock_path); > + g_free(sock_path); > + rmdir(sock_dir); > + > + g_assert_true(*sock_fd >= 0); > + > + return qts; > +} > + > void qtest_quit(QTestState *s) > { > g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, TRUE, s)); > diff --git a/tests/libqtest.h b/tests/libqtest.h > index 7ea94139b0..5937f91912 100644 > --- a/tests/libqtest.h > +++ b/tests/libqtest.h > @@ -62,6 +62,17 @@ QTestState *qtest_init(const char *extra_args); > */ > QTestState *qtest_init_without_qmp_handshake(const char *extra_args); > > +/** > + * qtest_init_with_serial: > + * @extra_args: other arguments to pass to QEMU. CAUTION: these > + * arguments are subject to word splitting and shell evaluation. > + * @sock_fd: pointer to store the socket file descriptor for > + * connection with serial. > + * > + * Returns: #QTestState instance. > + */ > +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd); > + > /** > * qtest_quit: > * @s: #QTestState instance to operate on. -- Alex Bennée
On 1/23/19 8:20 AM, Alex Bennée wrote: > > Julia Suvorova <jusual@mail.ru> writes: > >> Run qtest with a socket that connects QEMU chardev and test code. >> >> Signed-off-by: Julia Suvorova <jusual@mail.ru> >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > >> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd) >> +{ >> + int sock_fd_init; >> + char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX"; >> + QTestState *qts; >> + >> + g_assert_true(mkdtemp(sock_dir) != NULL); >> + sock_path = g_strdup_printf("%s/sock", sock_dir); >> + >> + sock_fd_init = init_socket(sock_path); >> + >> + qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s", >> + sock_path, extra_args); EWWWW. Please, let's not bake in this interface. Relying on shell splitting is nasty. >> >> +/** >> + * qtest_init_with_serial: >> + * @extra_args: other arguments to pass to QEMU. CAUTION: these >> + * arguments are subject to word splitting and shell evaluation. >> + * @sock_fd: pointer to store the socket file descriptor for >> + * connection with serial. >> + * >> + * Returns: #QTestState instance. >> + */ >> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd); I would MUCH rather see an interface that used varargs to pass extra arguments as a list/array of actual arguments, no shell splitting required.
On Wed, Jan 23, 2019 at 08:44:12AM -0600, Eric Blake wrote: > On 1/23/19 8:20 AM, Alex Bennée wrote: > > > > Julia Suvorova <jusual@mail.ru> writes: > > > >> Run qtest with a socket that connects QEMU chardev and test code. > >> > >> Signed-off-by: Julia Suvorova <jusual@mail.ru> > >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > > > > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > > > > >> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd) > >> +{ > >> + int sock_fd_init; > >> + char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX"; > >> + QTestState *qts; > >> + > >> + g_assert_true(mkdtemp(sock_dir) != NULL); > >> + sock_path = g_strdup_printf("%s/sock", sock_dir); > >> + > >> + sock_fd_init = init_socket(sock_path); > >> + > >> + qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s", > >> + sock_path, extra_args); > > EWWWW. Please, let's not bake in this interface. Relying on shell > splitting is nasty. qtest_initf() is long a standing interface that's relied on shell splitting for years :-( I agree we should fix that, but its a pre-existing problem that this patch doesn't make worse. Regards, Daniel
diff --git a/tests/libqtest.c b/tests/libqtest.c index 55750dd68d..6fb30855fa 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -315,6 +315,31 @@ QTestState *qtest_initf(const char *fmt, ...) return s; } +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd) +{ + int sock_fd_init; + char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX"; + QTestState *qts; + + g_assert_true(mkdtemp(sock_dir) != NULL); + sock_path = g_strdup_printf("%s/sock", sock_dir); + + sock_fd_init = init_socket(sock_path); + + qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s", + sock_path, extra_args); + + *sock_fd = socket_accept(sock_fd_init); + + unlink(sock_path); + g_free(sock_path); + rmdir(sock_dir); + + g_assert_true(*sock_fd >= 0); + + return qts; +} + void qtest_quit(QTestState *s) { g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, TRUE, s)); diff --git a/tests/libqtest.h b/tests/libqtest.h index 7ea94139b0..5937f91912 100644 --- a/tests/libqtest.h +++ b/tests/libqtest.h @@ -62,6 +62,17 @@ QTestState *qtest_init(const char *extra_args); */ QTestState *qtest_init_without_qmp_handshake(const char *extra_args); +/** + * qtest_init_with_serial: + * @extra_args: other arguments to pass to QEMU. CAUTION: these + * arguments are subject to word splitting and shell evaluation. + * @sock_fd: pointer to store the socket file descriptor for + * connection with serial. + * + * Returns: #QTestState instance. + */ +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd); + /** * qtest_quit: * @s: #QTestState instance to operate on.