Eric Blake <ebl...@redhat.com> writes:
> Now that the previous patches have fixed all callers to avoid an
> empty message, we can tweak qmp_fd_sendv() to assert that we
> don't introduce new callers, and reindent accordingly. The
> additional assertions will also help verify that later refactoring
> is not breaking anything.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> ---
> tests/libqtest.c | 38 ++++++++++++++++++--------------------
> 1 file changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 7e5425d704..99a07c246f 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -450,6 +450,9 @@ void qmp_fd_sendv(int fd, const char *fmt, va_list ap)
> {
> va_list ap_copy;
> QObject *qobj;
> + int log = getenv("QTEST_LOG") != NULL;
Use the opportunity to make this bool?
> + QString *qstr;
> + const char *str;
>
> /* qobject_from_jsonv() silently eats leading 0xff as invalid
> * JSON, but we want to test sending them over the wire to force
> @@ -458,6 +461,7 @@ void qmp_fd_sendv(int fd, const char *fmt, va_list ap)
> socket_send(fd, fmt, 1);
> fmt++;
> }
> + assert(*fmt);
I prefer assertions on arguments going first, for extra visibility.
> /* Going through qobject ensures we escape strings properly.
> * This seemingly unnecessary copy is required in case va_list
> @@ -466,29 +470,23 @@ void qmp_fd_sendv(int fd, const char *fmt, va_list ap)
> va_copy(ap_copy, ap);
> qobj = qobject_from_jsonv(fmt, &ap_copy, &error_abort);
> va_end(ap_copy);
> + qstr = qobject_to_json(qobj);
>
> - /* No need to send anything for an empty QObject. */
> - if (qobj) {
> - int log = getenv("QTEST_LOG") != NULL;
> - QString *qstr = qobject_to_json(qobj);
> - const char *str;
> + /*
> + * BUG: QMP doesn't react to input until it sees a newline, an
> + * object, or an array. Work-around: give it a newline.
> + */
> + qstring_append_chr(qstr, '\n');
> + str = qstring_get_str(qstr);
>
> - /*
> - * BUG: QMP doesn't react to input until it sees a newline, an
> - * object, or an array. Work-around: give it a newline.
> - */
> - qstring_append_chr(qstr, '\n');
> - str = qstring_get_str(qstr);
> -
> - if (log) {
> - fprintf(stderr, "%s", str);
> - }
> - /* Send QMP request */
> - socket_send(fd, str, qstring_get_length(qstr));
> -
> - QDECREF(qstr);
> - qobject_decref(qobj);
> + if (log) {
> + fprintf(stderr, "%s", str);
> }
> + /* Send QMP request */
> + socket_send(fd, str, qstring_get_length(qstr));
> +
> + QDECREF(qstr);
> + qobject_decref(qobj);
> }
>
> void qtest_async_qmpv(QTestState *s, const char *fmt, va_list ap)
Preferably with the assertion moved:
Reviewed-by: Markus Armbruster <arm...@redhat.com>
