All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libqtest: Rework qtest_rsp()
@ 2021-01-26 15:16 Markus Armbruster
  2021-01-26 16:10 ` Marc-André Lureau
  2021-01-27  6:16 ` Thomas Huth
  0 siblings, 2 replies; 3+ messages in thread
From: Markus Armbruster @ 2021-01-26 15:16 UTC (permalink / raw)
  To: qemu-devel; +Cc: lvivier, pbonzini, thuth

qtest_rsp() is used in two different ways: (1) return some arguments
to caller, which the caller must free, and (2) return no arguments to
caller.  Passing non-zero @expected_args gets you (1), and passing
zero gets you (2).

Having "the return value must be freed" depend on an argument this way
is less than ideal.

Provide separate functions for the two ways: (1) qtest_rsp_args()
takes @expected_args (possibly zero), and returns that number of
arguments.  Caller must free the return value always.  (2) qtest_rsp()
assumes zero, and returns nothing.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/qtest/libqtest.c | 50 ++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 5249a628cc..fd043b0570 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -503,7 +503,7 @@ static GString *qtest_client_socket_recv_line(QTestState *s)
     return line;
 }
 
-static gchar **qtest_rsp(QTestState *s, int expected_args)
+static gchar **qtest_rsp_args(QTestState *s, int expected_args)
 {
     GString *line;
     gchar **words;
@@ -539,25 +539,27 @@ redo:
     g_assert(words[0] != NULL);
     g_assert_cmpstr(words[0], ==, "OK");
 
-    if (expected_args) {
-        for (i = 0; i < expected_args; i++) {
-            g_assert(words[i] != NULL);
-        }
-    } else {
-        g_strfreev(words);
-        words = NULL;
+    for (i = 0; i < expected_args; i++) {
+        g_assert(words[i] != NULL);
     }
 
     return words;
 }
 
+static void qtest_rsp(QTestState *s)
+{
+    gchar **words = qtest_rsp_args(s, 0);
+
+    g_strfreev(words);
+}
+
 static int qtest_query_target_endianness(QTestState *s)
 {
     gchar **args;
     int big_endian;
 
     qtest_sendf(s, "endianness\n");
-    args = qtest_rsp(s, 1);
+    args = qtest_rsp_args(s, 1);
     g_assert(strcmp(args[1], "big") == 0 || strcmp(args[1], "little") == 0);
     big_endian = strcmp(args[1], "big") == 0;
     g_strfreev(args);
@@ -892,14 +894,14 @@ bool qtest_get_irq(QTestState *s, int num)
 void qtest_module_load(QTestState *s, const char *prefix, const char *libname)
 {
     qtest_sendf(s, "module_load %s %s\n", prefix, libname);
-    qtest_rsp(s, 0);
+    qtest_rsp(s);
 }
 
 static int64_t qtest_clock_rsp(QTestState *s)
 {
     gchar **words;
     int64_t clock;
-    words = qtest_rsp(s, 2);
+    words = qtest_rsp_args(s, 2);
     clock = g_ascii_strtoll(words[1], NULL, 0);
     g_strfreev(words);
     return clock;
@@ -926,13 +928,13 @@ int64_t qtest_clock_set(QTestState *s, int64_t val)
 void qtest_irq_intercept_out(QTestState *s, const char *qom_path)
 {
     qtest_sendf(s, "irq_intercept_out %s\n", qom_path);
-    qtest_rsp(s, 0);
+    qtest_rsp(s);
 }
 
 void qtest_irq_intercept_in(QTestState *s, const char *qom_path)
 {
     qtest_sendf(s, "irq_intercept_in %s\n", qom_path);
-    qtest_rsp(s, 0);
+    qtest_rsp(s);
 }
 
 void qtest_set_irq_in(QTestState *s, const char *qom_path, const char *name,
@@ -942,13 +944,13 @@ void qtest_set_irq_in(QTestState *s, const char *qom_path, const char *name,
         name = "unnamed-gpio-in";
     }
     qtest_sendf(s, "set_irq_in %s %s %d %d\n", qom_path, name, num, level);
-    qtest_rsp(s, 0);
+    qtest_rsp(s);
 }
 
 static void qtest_out(QTestState *s, const char *cmd, uint16_t addr, uint32_t value)
 {
     qtest_sendf(s, "%s 0x%x 0x%x\n", cmd, addr, value);
-    qtest_rsp(s, 0);
+    qtest_rsp(s);
 }
 
 void qtest_outb(QTestState *s, uint16_t addr, uint8_t value)
@@ -973,7 +975,7 @@ static uint32_t qtest_in(QTestState *s, const char *cmd, uint16_t addr)
     unsigned long value;
 
     qtest_sendf(s, "%s 0x%x\n", cmd, addr);
-    args = qtest_rsp(s, 2);
+    args = qtest_rsp_args(s, 2);
     ret = qemu_strtoul(args[1], NULL, 0, &value);
     g_assert(!ret && value <= UINT32_MAX);
     g_strfreev(args);
@@ -1000,7 +1002,7 @@ static void qtest_write(QTestState *s, const char *cmd, uint64_t addr,
                         uint64_t value)
 {
     qtest_sendf(s, "%s 0x%" PRIx64 " 0x%" PRIx64 "\n", cmd, addr, value);
-    qtest_rsp(s, 0);
+    qtest_rsp(s);
 }
 
 void qtest_writeb(QTestState *s, uint64_t addr, uint8_t value)
@@ -1030,7 +1032,7 @@ static uint64_t qtest_read(QTestState *s, const char *cmd, uint64_t addr)
     uint64_t value;
 
     qtest_sendf(s, "%s 0x%" PRIx64 "\n", cmd, addr);
-    args = qtest_rsp(s, 2);
+    args = qtest_rsp_args(s, 2);
     ret = qemu_strtou64(args[1], NULL, 0, &value);
     g_assert(!ret);
     g_strfreev(args);
@@ -1082,7 +1084,7 @@ void qtest_memread(QTestState *s, uint64_t addr, void *data, size_t size)
     }
 
     qtest_sendf(s, "read 0x%" PRIx64 " 0x%zx\n", addr, size);
-    args = qtest_rsp(s, 2);
+    args = qtest_rsp_args(s, 2);
 
     for (i = 0; i < size; i++) {
         ptr[i] = hex2nib(args[1][2 + (i * 2)]) << 4;
@@ -1098,7 +1100,7 @@ uint64_t qtest_rtas_call(QTestState *s, const char *name,
 {
     qtest_sendf(s, "rtas %s %u 0x%"PRIx64" %u 0x%"PRIx64"\n",
                 name, nargs, args, nret, ret);
-    qtest_rsp(s, 0);
+    qtest_rsp(s);
     return 0;
 }
 
@@ -1134,7 +1136,7 @@ void qtest_bufwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
     qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx ", addr, size);
     s->ops.send(s, bdata);
     s->ops.send(s, "\n");
-    qtest_rsp(s, 0);
+    qtest_rsp(s);
     g_free(bdata);
 }
 
@@ -1144,7 +1146,7 @@ void qtest_bufread(QTestState *s, uint64_t addr, void *data, size_t size)
     size_t len;
 
     qtest_sendf(s, "b64read 0x%" PRIx64 " 0x%zx\n", addr, size);
-    args = qtest_rsp(s, 2);
+    args = qtest_rsp_args(s, 2);
 
     g_base64_decode_inplace(args[1], &len);
     if (size != len) {
@@ -1174,14 +1176,14 @@ void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
     }
 
     qtest_sendf(s, "write 0x%" PRIx64 " 0x%zx 0x%s\n", addr, size, enc);
-    qtest_rsp(s, 0);
+    qtest_rsp(s);
     g_free(enc);
 }
 
 void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size)
 {
     qtest_sendf(s, "memset 0x%" PRIx64 " 0x%zx 0x%02x\n", addr, size, pattern);
-    qtest_rsp(s, 0);
+    qtest_rsp(s);
 }
 
 void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] libqtest: Rework qtest_rsp()
  2021-01-26 15:16 [PATCH] libqtest: Rework qtest_rsp() Markus Armbruster
@ 2021-01-26 16:10 ` Marc-André Lureau
  2021-01-27  6:16 ` Thomas Huth
  1 sibling, 0 replies; 3+ messages in thread
From: Marc-André Lureau @ 2021-01-26 16:10 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Laurent Vivier, Paolo Bonzini, Thomas Huth, QEMU

On Tue, Jan 26, 2021 at 7:19 PM Markus Armbruster <armbru@redhat.com> wrote:
>
> qtest_rsp() is used in two different ways: (1) return some arguments
> to caller, which the caller must free, and (2) return no arguments to
> caller.  Passing non-zero @expected_args gets you (1), and passing
> zero gets you (2).
>
> Having "the return value must be freed" depend on an argument this way
> is less than ideal.
>
> Provide separate functions for the two ways: (1) qtest_rsp_args()
> takes @expected_args (possibly zero), and returns that number of
> arguments.  Caller must free the return value always.  (2) qtest_rsp()
> assumes zero, and returns nothing.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/qtest/libqtest.c | 50 ++++++++++++++++++++++--------------------
>  1 file changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 5249a628cc..fd043b0570 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -503,7 +503,7 @@ static GString *qtest_client_socket_recv_line(QTestState *s)
>      return line;
>  }
>
> -static gchar **qtest_rsp(QTestState *s, int expected_args)
> +static gchar **qtest_rsp_args(QTestState *s, int expected_args)
>  {
>      GString *line;
>      gchar **words;
> @@ -539,25 +539,27 @@ redo:
>      g_assert(words[0] != NULL);
>      g_assert_cmpstr(words[0], ==, "OK");
>
> -    if (expected_args) {
> -        for (i = 0; i < expected_args; i++) {
> -            g_assert(words[i] != NULL);
> -        }
> -    } else {
> -        g_strfreev(words);
> -        words = NULL;
> +    for (i = 0; i < expected_args; i++) {
> +        g_assert(words[i] != NULL);
>      }
>
>      return words;
>  }
>
> +static void qtest_rsp(QTestState *s)
> +{
> +    gchar **words = qtest_rsp_args(s, 0);

You may even use g_auto(GStrv) if you fancy that.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> +
> +    g_strfreev(words);
> +}
> +
>  static int qtest_query_target_endianness(QTestState *s)
>  {
>      gchar **args;
>      int big_endian;
>
>      qtest_sendf(s, "endianness\n");
> -    args = qtest_rsp(s, 1);
> +    args = qtest_rsp_args(s, 1);
>      g_assert(strcmp(args[1], "big") == 0 || strcmp(args[1], "little") == 0);
>      big_endian = strcmp(args[1], "big") == 0;
>      g_strfreev(args);
> @@ -892,14 +894,14 @@ bool qtest_get_irq(QTestState *s, int num)
>  void qtest_module_load(QTestState *s, const char *prefix, const char *libname)
>  {
>      qtest_sendf(s, "module_load %s %s\n", prefix, libname);
> -    qtest_rsp(s, 0);
> +    qtest_rsp(s);
>  }
>
>  static int64_t qtest_clock_rsp(QTestState *s)
>  {
>      gchar **words;
>      int64_t clock;
> -    words = qtest_rsp(s, 2);
> +    words = qtest_rsp_args(s, 2);
>      clock = g_ascii_strtoll(words[1], NULL, 0);
>      g_strfreev(words);
>      return clock;
> @@ -926,13 +928,13 @@ int64_t qtest_clock_set(QTestState *s, int64_t val)
>  void qtest_irq_intercept_out(QTestState *s, const char *qom_path)
>  {
>      qtest_sendf(s, "irq_intercept_out %s\n", qom_path);
> -    qtest_rsp(s, 0);
> +    qtest_rsp(s);
>  }
>
>  void qtest_irq_intercept_in(QTestState *s, const char *qom_path)
>  {
>      qtest_sendf(s, "irq_intercept_in %s\n", qom_path);
> -    qtest_rsp(s, 0);
> +    qtest_rsp(s);
>  }
>
>  void qtest_set_irq_in(QTestState *s, const char *qom_path, const char *name,
> @@ -942,13 +944,13 @@ void qtest_set_irq_in(QTestState *s, const char *qom_path, const char *name,
>          name = "unnamed-gpio-in";
>      }
>      qtest_sendf(s, "set_irq_in %s %s %d %d\n", qom_path, name, num, level);
> -    qtest_rsp(s, 0);
> +    qtest_rsp(s);
>  }
>
>  static void qtest_out(QTestState *s, const char *cmd, uint16_t addr, uint32_t value)
>  {
>      qtest_sendf(s, "%s 0x%x 0x%x\n", cmd, addr, value);
> -    qtest_rsp(s, 0);
> +    qtest_rsp(s);
>  }
>
>  void qtest_outb(QTestState *s, uint16_t addr, uint8_t value)
> @@ -973,7 +975,7 @@ static uint32_t qtest_in(QTestState *s, const char *cmd, uint16_t addr)
>      unsigned long value;
>
>      qtest_sendf(s, "%s 0x%x\n", cmd, addr);
> -    args = qtest_rsp(s, 2);
> +    args = qtest_rsp_args(s, 2);
>      ret = qemu_strtoul(args[1], NULL, 0, &value);
>      g_assert(!ret && value <= UINT32_MAX);
>      g_strfreev(args);
> @@ -1000,7 +1002,7 @@ static void qtest_write(QTestState *s, const char *cmd, uint64_t addr,
>                          uint64_t value)
>  {
>      qtest_sendf(s, "%s 0x%" PRIx64 " 0x%" PRIx64 "\n", cmd, addr, value);
> -    qtest_rsp(s, 0);
> +    qtest_rsp(s);
>  }
>
>  void qtest_writeb(QTestState *s, uint64_t addr, uint8_t value)
> @@ -1030,7 +1032,7 @@ static uint64_t qtest_read(QTestState *s, const char *cmd, uint64_t addr)
>      uint64_t value;
>
>      qtest_sendf(s, "%s 0x%" PRIx64 "\n", cmd, addr);
> -    args = qtest_rsp(s, 2);
> +    args = qtest_rsp_args(s, 2);
>      ret = qemu_strtou64(args[1], NULL, 0, &value);
>      g_assert(!ret);
>      g_strfreev(args);
> @@ -1082,7 +1084,7 @@ void qtest_memread(QTestState *s, uint64_t addr, void *data, size_t size)
>      }
>
>      qtest_sendf(s, "read 0x%" PRIx64 " 0x%zx\n", addr, size);
> -    args = qtest_rsp(s, 2);
> +    args = qtest_rsp_args(s, 2);
>
>      for (i = 0; i < size; i++) {
>          ptr[i] = hex2nib(args[1][2 + (i * 2)]) << 4;
> @@ -1098,7 +1100,7 @@ uint64_t qtest_rtas_call(QTestState *s, const char *name,
>  {
>      qtest_sendf(s, "rtas %s %u 0x%"PRIx64" %u 0x%"PRIx64"\n",
>                  name, nargs, args, nret, ret);
> -    qtest_rsp(s, 0);
> +    qtest_rsp(s);
>      return 0;
>  }
>
> @@ -1134,7 +1136,7 @@ void qtest_bufwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
>      qtest_sendf(s, "b64write 0x%" PRIx64 " 0x%zx ", addr, size);
>      s->ops.send(s, bdata);
>      s->ops.send(s, "\n");
> -    qtest_rsp(s, 0);
> +    qtest_rsp(s);
>      g_free(bdata);
>  }
>
> @@ -1144,7 +1146,7 @@ void qtest_bufread(QTestState *s, uint64_t addr, void *data, size_t size)
>      size_t len;
>
>      qtest_sendf(s, "b64read 0x%" PRIx64 " 0x%zx\n", addr, size);
> -    args = qtest_rsp(s, 2);
> +    args = qtest_rsp_args(s, 2);
>
>      g_base64_decode_inplace(args[1], &len);
>      if (size != len) {
> @@ -1174,14 +1176,14 @@ void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
>      }
>
>      qtest_sendf(s, "write 0x%" PRIx64 " 0x%zx 0x%s\n", addr, size, enc);
> -    qtest_rsp(s, 0);
> +    qtest_rsp(s);
>      g_free(enc);
>  }
>
>  void qtest_memset(QTestState *s, uint64_t addr, uint8_t pattern, size_t size)
>  {
>      qtest_sendf(s, "memset 0x%" PRIx64 " 0x%zx 0x%02x\n", addr, size, pattern);
> -    qtest_rsp(s, 0);
> +    qtest_rsp(s);
>  }
>
>  void qtest_qmp_assert_success(QTestState *qts, const char *fmt, ...)
> --
> 2.26.2
>
>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] libqtest: Rework qtest_rsp()
  2021-01-26 15:16 [PATCH] libqtest: Rework qtest_rsp() Markus Armbruster
  2021-01-26 16:10 ` Marc-André Lureau
@ 2021-01-27  6:16 ` Thomas Huth
  1 sibling, 0 replies; 3+ messages in thread
From: Thomas Huth @ 2021-01-27  6:16 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: lvivier, pbonzini

On 26/01/2021 16.16, Markus Armbruster wrote:
> qtest_rsp() is used in two different ways: (1) return some arguments
> to caller, which the caller must free, and (2) return no arguments to
> caller.  Passing non-zero @expected_args gets you (1), and passing
> zero gets you (2).
> 
> Having "the return value must be freed" depend on an argument this way
> is less than ideal.
> 
> Provide separate functions for the two ways: (1) qtest_rsp_args()
> takes @expected_args (possibly zero), and returns that number of
> arguments.  Caller must free the return value always.  (2) qtest_rsp()
> assumes zero, and returns nothing.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   tests/qtest/libqtest.c | 50 ++++++++++++++++++++++--------------------
>   1 file changed, 26 insertions(+), 24 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-01-27  6:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-26 15:16 [PATCH] libqtest: Rework qtest_rsp() Markus Armbruster
2021-01-26 16:10 ` Marc-André Lureau
2021-01-27  6:16 ` Thomas Huth

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.