Re: [Qemu-devel] [PATCH v2 2/3] qtest: extend qtest_qmp() to fill in the reply

[Anthony Liguori <aliguori@us.ibm.com>]

Jason Baron <jbaron@redhat.com> writes:

> From: Jason Baron <jbaron@redhat.com>
>
> Introduce:
>
> Add void qtest_qmp_resp(QTestState *s, QString *resp, const char *fmt,
> ...)

Any reason to not just return a QString?

Also, why not do the parsing in this function and just return a QObject
of the result?

Regards,

Anthony Liguori

>
> which allows a response string to be filled in.
>
> Signed-off-by: Jason Baron <jbaron@redhat.com>
> ---
>  tests/Makefile   |    2 +-
>  tests/libqtest.c |   17 ++++++++++-------
>  tests/libqtest.h |   15 +++++++++++++--
>  3 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/tests/Makefile b/tests/Makefile
> index b60f0fb..1756b47 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -85,7 +85,7 @@ TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_DIRS)))
>  QTEST_TARGETS=$(foreach TARGET,$(TARGETS), $(if $(check-qtest-$(TARGET)-y), $(TARGET),))
>  check-qtest-$(CONFIG_POSIX)=$(foreach TARGET,$(TARGETS), $(check-qtest-$(TARGET)-y))
>  
> -qtest-obj-y = tests/libqtest.o $(oslib-obj-y) libqemustub.a
> +qtest-obj-y = tests/libqtest.o $(oslib-obj-y) libqemustub.a $(test-qapi-obj-y) $(qom-obj-y)
>  $(check-qtest-y): $(qtest-obj-y)
>  
>  .PHONY: check-help
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 57665c9..994cd2f 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -288,7 +288,7 @@ redo:
>      return words;
>  }
>  
> -void qtest_qmp(QTestState *s, const char *fmt, ...)
> +void qtest_qmp_resp(QTestState *s, QString *resp, const char *fmt, ...)
>  {
>      va_list ap;
>      bool has_reply = false;
> @@ -313,16 +313,19 @@ void qtest_qmp(QTestState *s, const char *fmt, ...)
>              fprintf(stderr, "Broken pipe\n");
>              exit(1);
>          }
> -
> -        switch (c) {
> -        case '{':
> +        if (c == '{') {
>              nesting++;
>              has_reply = true;
> -            break;
> -        case '}':
> +        }
> +        if (c == '}') {
>              nesting--;
> -            break;
>          }
> +        if (has_reply && resp) {
> +            qstring_append_chr(resp, c);
> +        }
> +    }
> +    if (has_reply && resp) {
> +        qstring_append_chr(resp, '\0');
>      }
>  }
>  
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index c8ade85..972ba5d 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -18,6 +18,7 @@
>  #include <stdint.h>
>  #include <stdbool.h>
>  #include <sys/types.h>
> +#include "qstring.h"
>  
>  typedef struct QTestState QTestState;
>  
> @@ -38,13 +39,14 @@ QTestState *qtest_init(const char *extra_args);
>  void qtest_quit(QTestState *s);
>  
>  /**
> - * qtest_qmp:
> + * qtest_qmp_resp:
>   * @s: QTestState instance to operate on.
> + * @resp: Fills in response string if provided
>   * @fmt...: QMP message to send to qemu
>   *
>   * Sends a QMP message to QEMU
>   */
> -void qtest_qmp(QTestState *s, const char *fmt, ...);
> +void qtest_qmp_resp(QTestState *s, QString *resp, const char *fmt, ...);
>  
>  /**
>   * qtest_get_irq:
> @@ -349,4 +351,13 @@ void qtest_add_func(const char *str, void (*fn));
>   */
>  #define clock_set(val) qtest_clock_set(global_qtest, val)
>  
> +/**
> + * qtest_qmp:
> + * @s: QTestState instance to operate on.
> + * @fmt...: QMP message to send to qemu
> + *
> + * Sends a QMP message to QEMU
> + */
> +#define qtest_qmp(s, fmt, ...) qtest_qmp_resp(s, NULL, fmt, ## __VA_ARGS__)
> +
>  #endif
> -- 
> 1.7.1