[PATCH 0/4] vl: avoid SIGSEGV on invalid [accel] configuration

[PATCH 0/4] vl: avoid SIGSEGV on invalid [accel] configuration

[Paolo Bonzini]

While QEMU catches invalid -accel command line options:

    $ qemu-system-x86_64 -accel foo=bar
    Accelerators supported in QEMU binary:
    tcg
    xen
    kvm

the same is not true of configuration files, which instead crash.  Patch 1
is the trivial fix, but writing a test is a bit more complex: there are
no existing testcases where the qtest socket would not even start.  So
the series does the required refactoring and cleanup before adding tests
for both valid and invalid [accel] sections in patch 4.

Paolo Bonzini (4):
  vl: catch [accel] entry without accelerator
  libqtest: split qtest_spawn_qemu function
  libqtest: ensure waitpid() is only called once
  readconfig-test: add test for accelerator configuration

 softmmu/vl.c                  |  15 +++-
 tests/qtest/libqtest.c        | 160 +++++++++++++++++++++-------------
 tests/qtest/libqtest.h        |  12 +++
 tests/qtest/readconfig-test.c |  45 ++++++++--
 4 files changed, 156 insertions(+), 76 deletions(-)

-- 
2.38.1

[PATCH 1/4] vl: catch [accel] entry without accelerator

[Paolo Bonzini]

While QEMU catches invalid -accel command line options:

    $ qemu-system-x86_64 -accel foo=bar
    Accelerators supported in QEMU binary:
    tcg
    xen
    kvm

the same is not true of configuration files, which instead crash.
Avoid a SIGSEGV and return an error instead.

Reported-by: Thomas Huth <thuth@redhat.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1439
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 softmmu/vl.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 9bd0e52d016a..b6deaee52da4 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2204,14 +2204,18 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
     int ret;
     bool qtest_with_kvm;
 
+    if (!acc) {
+        error_setg(&error_fatal, QERR_MISSING_PARAMETER, "accel");
+        goto bad;
+    }
+
     qtest_with_kvm = g_str_equal(acc, "kvm") && qtest_chrdev != NULL;
 
     if (!ac) {
-        *p_init_failed = true;
         if (!qtest_with_kvm) {
             error_report("invalid accelerator %s", acc);
         }
-        return 0;
+        goto bad;
     }
     accel = ACCEL(object_new_with_class(OBJECT_CLASS(ac)));
     object_apply_compat_props(OBJECT(accel));
@@ -2221,14 +2225,17 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
 
     ret = accel_init_machine(accel, current_machine);
     if (ret < 0) {
-        *p_init_failed = true;
         if (!qtest_with_kvm || ret != -ENOENT) {
             error_report("failed to initialize %s: %s", acc, strerror(-ret));
         }
-        return 0;
+        goto bad;
     }
 
     return 1;
+
+bad:
+    *p_init_failed = true;
+    return 0;
 }
 
 static void configure_accelerators(const char *progname)
-- 
2.38.1

Re: [PATCH 1/4] vl: catch [accel] entry without accelerator

[Philippe Mathieu-Daudé]

On 17/1/23 09:07, Paolo Bonzini wrote:
> While QEMU catches invalid -accel command line options:
> 
>      $ qemu-system-x86_64 -accel foo=bar
>      Accelerators supported in QEMU binary:
>      tcg
>      xen
>      kvm
> 
> the same is not true of configuration files, which instead crash.
> Avoid a SIGSEGV and return an error instead.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1439
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   softmmu/vl.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 9bd0e52d016a..b6deaee52da4 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2204,14 +2204,18 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
>       int ret;
>       bool qtest_with_kvm;
>   
> +    if (!acc) {
> +        error_setg(&error_fatal, QERR_MISSING_PARAMETER, "accel");

s/&error_fatal/errp/ ?

> +        goto bad;
> +    }
> +
>       qtest_with_kvm = g_str_equal(acc, "kvm") && qtest_chrdev != NULL;
>   
>       if (!ac) {
> -        *p_init_failed = true;
>           if (!qtest_with_kvm) {
>               error_report("invalid accelerator %s", acc);
>           }
> -        return 0;
> +        goto bad;
>       }
>       accel = ACCEL(object_new_with_class(OBJECT_CLASS(ac)));
>       object_apply_compat_props(OBJECT(accel));
> @@ -2221,14 +2225,17 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
>   
>       ret = accel_init_machine(accel, current_machine);
>       if (ret < 0) {
> -        *p_init_failed = true;
>           if (!qtest_with_kvm || ret != -ENOENT) {
>               error_report("failed to initialize %s: %s", acc, strerror(-ret));
>           }
> -        return 0;
> +        goto bad;
>       }
>   
>       return 1;
> +
> +bad:
> +    *p_init_failed = true;
> +    return 0;
>   }

[PATCH 2/4] libqtest: split qtest_spawn_qemu function

[Paolo Bonzini]

In order to create a function that allows testing of invalid command
lines, extract the parts of qtest_init_without_qmp_handshake that do
not require any successful set up of sockets.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/libqtest.c | 103 ++++++++++++++++++++++-------------------
 1 file changed, 55 insertions(+), 48 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 5cb38f90da19..4d9cf919b2f7 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -342,60 +342,25 @@ static pid_t qtest_create_process(char *cmd)
 }
 #endif /* _WIN32 */
 
-QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
+static QTestState *G_GNUC_PRINTF(1, 0) qtest_spawn_qemu(const char *fmt, ...)
 {
-    QTestState *s;
-    int sock, qmpsock, i;
-    gchar *socket_path;
-    gchar *qmp_socket_path;
-    gchar *command;
-    const char *qemu_binary = qtest_qemu_binary();
+    va_list ap;
+    QTestState *s = g_new0(QTestState, 1);
     const char *trace = g_getenv("QTEST_TRACE");
     g_autofree char *tracearg = trace ?
         g_strdup_printf("-trace %s ", trace) : g_strdup("");
+    g_autoptr(GString) command = g_string_new("");
 
-    s = g_new(QTestState, 1);
-
-    socket_path = g_strdup_printf("%s/qtest-%d.sock",
-                                  g_get_tmp_dir(), getpid());
-    qmp_socket_path = g_strdup_printf("%s/qtest-%d.qmp",
-                                      g_get_tmp_dir(), getpid());
-
-    /* It's possible that if an earlier test run crashed it might
-     * have left a stale unix socket lying around. Delete any
-     * stale old socket to avoid spurious test failures with
-     * tests/libqtest.c:70:init_socket: assertion failed (ret != -1): (-1 != -1)
-     */
-    unlink(socket_path);
-    unlink(qmp_socket_path);
-
-    socket_init();
-    sock = init_socket(socket_path);
-    qmpsock = init_socket(qmp_socket_path);
-
-    qtest_client_set_rx_handler(s, qtest_client_socket_recv_line);
-    qtest_client_set_tx_handler(s, qtest_client_socket_send);
+    va_start(ap, fmt);
+    g_string_append_printf(command, CMD_EXEC "%s %s",
+                           qtest_qemu_binary(), tracearg);
+    g_string_append_vprintf(command, fmt, ap);
+    va_end(ap);
 
     qtest_add_abrt_handler(kill_qemu_hook_func, s);
 
-    command = g_strdup_printf(CMD_EXEC "%s %s"
-                              "-qtest unix:%s "
-                              "-qtest-log %s "
-                              "-chardev socket,path=%s,id=char0 "
-                              "-mon chardev=char0,mode=control "
-                              "-display none "
-                              "%s"
-                              " -accel qtest",
-                              qemu_binary, tracearg, socket_path,
-                              getenv("QTEST_LOG") ? DEV_STDERR : DEV_NULL,
-                              qmp_socket_path,
-                              extra_args ?: "");
+    g_test_message("starting QEMU: %s", command->str);
 
-    g_test_message("starting QEMU: %s", command);
-
-    s->pending_events = NULL;
-    s->wstatus = 0;
-    s->expected_status = 0;
 #ifndef _WIN32
     s->qemu_pid = fork();
     if (s->qemu_pid == 0) {
@@ -416,14 +381,56 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
         if (!g_setenv("QEMU_AUDIO_DRV", "none", true)) {
             exit(1);
         }
-        execlp("/bin/sh", "sh", "-c", command, NULL);
+        execlp("/bin/sh", "sh", "-c", command->str, NULL);
         exit(1);
     }
 #else
-    s->qemu_pid = qtest_create_process(command);
+    s->qemu_pid = qtest_create_process(command->str);
 #endif /* _WIN32 */
 
-    g_free(command);
+    return s;
+}
+
+QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
+{
+    QTestState *s;
+    int sock, qmpsock, i;
+    gchar *socket_path;
+    gchar *qmp_socket_path;
+
+    socket_path = g_strdup_printf("%s/qtest-%d.sock",
+                                  g_get_tmp_dir(), getpid());
+    qmp_socket_path = g_strdup_printf("%s/qtest-%d.qmp",
+                                      g_get_tmp_dir(), getpid());
+
+    /*
+     * It's possible that if an earlier test run crashed it might
+     * have left a stale unix socket lying around. Delete any
+     * stale old socket to avoid spurious test failures with
+     * tests/libqtest.c:70:init_socket: assertion failed (ret != -1): (-1 != -1)
+     */
+    unlink(socket_path);
+    unlink(qmp_socket_path);
+
+    socket_init();
+    sock = init_socket(socket_path);
+    qmpsock = init_socket(qmp_socket_path);
+
+    s = qtest_spawn_qemu("-qtest unix:%s "
+                         "-qtest-log %s "
+                         "-chardev socket,path=%s,id=char0 "
+                         "-mon chardev=char0,mode=control "
+                         "-display none "
+                         "%s"
+                         " -accel qtest",
+                         socket_path,
+                         getenv("QTEST_LOG") ? DEV_STDERR : DEV_NULL,
+                         qmp_socket_path,
+                         extra_args ?: "");
+
+    qtest_client_set_rx_handler(s, qtest_client_socket_recv_line);
+    qtest_client_set_tx_handler(s, qtest_client_socket_send);
+
     s->fd = socket_accept(sock);
     if (s->fd >= 0) {
         s->qmp_fd = socket_accept(qmpsock);
-- 
2.38.1

Re: [PATCH 2/4] libqtest: split qtest_spawn_qemu function

[Philippe Mathieu-Daudé]

On 17/1/23 09:07, Paolo Bonzini wrote:
> In order to create a function that allows testing of invalid command
> lines, extract the parts of qtest_init_without_qmp_handshake that do
> not require any successful set up of sockets.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   tests/qtest/libqtest.c | 103 ++++++++++++++++++++++-------------------
>   1 file changed, 55 insertions(+), 48 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 2/4] libqtest: split qtest_spawn_qemu function

[Thomas Huth]

On 17/01/2023 09.07, Paolo Bonzini wrote:
> In order to create a function that allows testing of invalid command
> lines, extract the parts of qtest_init_without_qmp_handshake that do
> not require any successful set up of sockets.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   tests/qtest/libqtest.c | 103 ++++++++++++++++++++++-------------------
>   1 file changed, 55 insertions(+), 48 deletions(-)
> 
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 5cb38f90da19..4d9cf919b2f7 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -342,60 +342,25 @@ static pid_t qtest_create_process(char *cmd)
>   }
>   #endif /* _WIN32 */
>   
> -QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
> +static QTestState *G_GNUC_PRINTF(1, 0) qtest_spawn_qemu(const char *fmt, ...)

Gluing the "*" to G_GNUC_PRINTF looks weird, could you maybe put that 
G_GNUC_PRINTF somewhere else?

  Thomas

[PATCH 3/4] libqtest: ensure waitpid() is only called once

[Paolo Bonzini]

If a test aborts after qtest_wait_qemu() is called, the SIGABRT hooks are
still in place and waitpid() is called again.  The second time it is called,
the process does not exist anymore and the system call fails.

Move the s->qemu_pid = -1 assignment to qtest_wait_qemu() to make it
idempotent, and anyway remove the SIGABRT hook as well to avoid that
qtest_check_status() is called twice.  Because of the extra call,
qtest_remove_abrt_handler() now has to be made idempotent as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/libqtest.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 4d9cf919b2f7..64ba98bc5853 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -156,6 +156,7 @@ bool qtest_probe_child(QTestState *s)
         CloseHandle((HANDLE)pid);
 #endif
         s->qemu_pid = -1;
+        qtest_remove_abrt_handler(s);
     }
     return false;
 }
@@ -167,6 +168,8 @@ void qtest_set_expected_status(QTestState *s, int status)
 
 static void qtest_check_status(QTestState *s)
 {
+    assert (s->qemu_pid == -1);
+
     /*
      * Check whether qemu exited with expected exit status; anything else is
      * fishy and should be logged with as much detail as possible.
@@ -200,20 +203,24 @@ static void qtest_check_status(QTestState *s)
 
 void qtest_wait_qemu(QTestState *s)
 {
+    if (s->qemu_pid != -1) {
 #ifndef _WIN32
-    pid_t pid;
+        pid_t pid;
 
-    pid = RETRY_ON_EINTR(waitpid(s->qemu_pid, &s->wstatus, 0));
-    assert(pid == s->qemu_pid);
+        pid = RETRY_ON_EINTR(waitpid(s->qemu_pid, &s->wstatus, 0));
+        g_assert_cmpint(pid, ==, s->qemu_pid);
 #else
-    DWORD ret;
+        DWORD ret;
 
-    ret = WaitForSingleObject((HANDLE)s->qemu_pid, INFINITE);
-    assert(ret == WAIT_OBJECT_0);
-    GetExitCodeProcess((HANDLE)s->qemu_pid, &s->exit_code);
-    CloseHandle((HANDLE)s->qemu_pid);
+        ret = WaitForSingleObject((HANDLE)s->qemu_pid, INFINITE);
+        assert(ret == WAIT_OBJECT_0);
+        GetExitCodeProcess((HANDLE)s->qemu_pid, &s->exit_code);
+        CloseHandle((HANDLE)s->qemu_pid);
 #endif
 
+        s->qemu_pid = -1;
+        qtest_remove_abrt_handler(s);
+    }
     qtest_check_status(s);
 }
 
@@ -227,7 +234,6 @@ void qtest_kill_qemu(QTestState *s)
         TerminateProcess((HANDLE)s->qemu_pid, s->expected_status);
 #endif
         qtest_wait_qemu(s);
-        s->qemu_pid = -1;
         return;
     }
 
@@ -289,6 +295,11 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data)
 void qtest_remove_abrt_handler(void *data)
 {
     GHook *hook = g_hook_find_data(&abrt_hooks, TRUE, data);
+
+    if (!hook) {
+        return;
+    }
+
     g_hook_destroy_link(&abrt_hooks, hook);
 
     /* Uninstall SIGABRT handler on last instance */
-- 
2.38.1

Re: [PATCH 3/4] libqtest: ensure waitpid() is only called once

[Thomas Huth]

On 17/01/2023 09.07, Paolo Bonzini wrote:
> If a test aborts after qtest_wait_qemu() is called, the SIGABRT hooks are
> still in place and waitpid() is called again.  The second time it is called,
> the process does not exist anymore and the system call fails.
> 
> Move the s->qemu_pid = -1 assignment to qtest_wait_qemu() to make it
> idempotent, and anyway remove the SIGABRT hook as well to avoid that
> qtest_check_status() is called twice.  Because of the extra call,
> qtest_remove_abrt_handler() now has to be made idempotent as well.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   tests/qtest/libqtest.c | 29 ++++++++++++++++++++---------
>   1 file changed, 20 insertions(+), 9 deletions(-)

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

[PATCH 4/4] readconfig-test: add test for accelerator configuration

[Paolo Bonzini]

Test that invalid configurations do not cause a SIGSEGV, and cover a
valid configuration as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/qtest/libqtest.c        | 28 +++++++++++++++++-----
 tests/qtest/libqtest.h        | 12 ++++++++++
 tests/qtest/readconfig-test.c | 45 ++++++++++++++++++++++++++++-------
 3 files changed, 70 insertions(+), 15 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 64ba98bc5853..53d766fe3fa5 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -402,6 +402,26 @@ static QTestState *G_GNUC_PRINTF(1, 0) qtest_spawn_qemu(const char *fmt, ...)
     return s;
 }
 
+QTestState *G_GNUC_PRINTF(1, 0) qtest_init_bare(const char *args)
+{
+    QTestState *s = qtest_spawn_qemu("%s", args);
+
+    /*
+     * Stopping QEMU for debugging is not supported on Windows.
+     *
+     * Using DebugActiveProcess() API can suspend the QEMU process,
+     * but gdb cannot attach to the process. Using the undocumented
+     * NtSuspendProcess() can suspend the QEMU process and gdb can
+     * attach to the process, but gdb cannot resume it.
+     */
+#ifndef _WIN32
+    if (getenv("QTEST_STOP")) {
+        kill(s->qemu_pid, SIGSTOP);
+    }
+#endif
+    return s;
+}
+
 QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
 {
     QTestState *s;
@@ -459,12 +479,8 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     }
 
     /*
-     * Stopping QEMU for debugging is not supported on Windows.
-     *
-     * Using DebugActiveProcess() API can suspend the QEMU process,
-     * but gdb cannot attach to the process. Using the undocumented
-     * NtSuspendProcess() can suspend the QEMU process and gdb can
-     * attach to the process, but gdb cannot resume it.
+     * Stopping QEMU for debugging is not supported on Windows;
+     * see qtest_init_bare for more information.
      */
 #ifndef _WIN32
     if (getenv("QTEST_STOP")) {
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index fcf1c3c3b36f..7ca7df26a2c0 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -23,6 +23,18 @@
 
 typedef struct QTestState QTestState;
 
+/**
+ * qtest_init_bare:
+ * @extra_args: other arguments to pass to QEMU.  CAUTION: these
+ * arguments are subject to word splitting and shell evaluation.
+ *
+ * Return a QTestState instance without automatically creating any
+ * sockets for QMP and qtest communication.
+ *
+ * Returns: #QTestState instance.
+ */
+QTestState *qtest_init_bare(const char *args);
+
 /**
  * qtest_initf:
  * @fmt: Format for creating other arguments to pass to QEMU, formatted
diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c
index 9ef870643dcd..4c11883e36eb 100644
--- a/tests/qtest/readconfig-test.c
+++ b/tests/qtest/readconfig-test.c
@@ -19,13 +19,11 @@
 #include "qapi/qmp/qstring.h"
 #include "qemu/units.h"
 
-static QTestState *qtest_init_with_config(const char *cfgdata)
+static char *qtest_write_config(const char *cfgdata)
 {
     GError *error = NULL;
-    g_autofree char *args = NULL;
     int cfgfd = -1;
-    g_autofree char *cfgpath = NULL;
-    QTestState *qts;
+    char *cfgpath;
     ssize_t ret;
 
     cfgfd = g_file_open_tmp("readconfig-test-XXXXXX", &cfgpath, &error);
@@ -38,13 +36,14 @@ static QTestState *qtest_init_with_config(const char *cfgdata)
         unlink(cfgpath);
     }
     g_assert_cmpint(ret, ==, strlen(cfgdata));
+    return cfgpath;
+}
 
-    args = g_strdup_printf("-nodefaults -machine none -readconfig %s", cfgpath);
-
-    qts = qtest_init(args);
-
+static QTestState *qtest_init_with_config(const char *cfgdata)
+{
+    g_autofree char *cfgpath = qtest_write_config(cfgdata);
+    QTestState *qts = qtest_initf("-nodefaults -machine none -readconfig %s", cfgpath);
     unlink(cfgpath);
-
     return qts;
 }
 
@@ -176,6 +175,32 @@ static void test_object_rng(void)
     qtest_quit(qts);
 }
 
+static void test_invalid_accel(void)
+{
+    const char *cfgdata =
+        "[accel]\n"
+        "foo = \"bar\"\n";
+
+    g_autofree char *cfgpath = qtest_write_config(cfgdata);
+    g_autofree char *args = g_strdup_printf("-nodefaults -machine none -readconfig %s", cfgpath);
+    QTestState *qts = qtest_init_bare(args);
+
+    qtest_set_expected_status(qts, 1);
+    qtest_wait_qemu(qts);
+    g_free(qts);
+    unlink(cfgpath);
+}
+
+static void test_valid_accel(void)
+{
+    const char *cfgdata =
+        "[accel]\n"
+        "accel = \"qtest\"\n";
+
+    QTestState *qts = qtest_init_with_config(cfgdata);
+    qtest_quit(qts);
+}
+
 int main(int argc, char *argv[])
 {
     const char *arch;
@@ -192,6 +217,8 @@ int main(int argc, char *argv[])
 #endif
 
     qtest_add_func("readconfig/object-rng", test_object_rng);
+    qtest_add_func("readconfig/invalid-accel", test_invalid_accel);
+    qtest_add_func("readconfig/valid-accel", test_valid_accel);
 
     return g_test_run();
 }
-- 
2.38.1

Re: [PATCH 4/4] readconfig-test: add test for accelerator configuration

[Thomas Huth]

On 17/01/2023 09.07, Paolo Bonzini wrote:
> Test that invalid configurations do not cause a SIGSEGV, and cover a
> valid configuration as well.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   tests/qtest/libqtest.c        | 28 +++++++++++++++++-----
>   tests/qtest/libqtest.h        | 12 ++++++++++
>   tests/qtest/readconfig-test.c | 45 ++++++++++++++++++++++++++++-------
>   3 files changed, 70 insertions(+), 15 deletions(-)
> 
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 64ba98bc5853..53d766fe3fa5 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -402,6 +402,26 @@ static QTestState *G_GNUC_PRINTF(1, 0) qtest_spawn_qemu(const char *fmt, ...)
>       return s;
>   }
>   
> +QTestState *G_GNUC_PRINTF(1, 0) qtest_init_bare(const char *args)

I think you don't need the G_GNUC_PRINTF here, do you?

  Thomas

Re: [PATCH 4/4] readconfig-test: add test for accelerator configuration

[Richard Henderson]

On 1/31/23 05:18, Thomas Huth wrote:
> On 17/01/2023 09.07, Paolo Bonzini wrote:
>> Test that invalid configurations do not cause a SIGSEGV, and cover a
>> valid configuration as well.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   tests/qtest/libqtest.c        | 28 +++++++++++++++++-----
>>   tests/qtest/libqtest.h        | 12 ++++++++++
>>   tests/qtest/readconfig-test.c | 45 ++++++++++++++++++++++++++++-------
>>   3 files changed, 70 insertions(+), 15 deletions(-)
>>
>> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
>> index 64ba98bc5853..53d766fe3fa5 100644
>> --- a/tests/qtest/libqtest.c
>> +++ b/tests/qtest/libqtest.c
>> @@ -402,6 +402,26 @@ static QTestState *G_GNUC_PRINTF(1, 0) qtest_spawn_qemu(const char 
>> *fmt, ...)
>>       return s;
>>   }
>> +QTestState *G_GNUC_PRINTF(1, 0) qtest_init_bare(const char *args)
> 
> I think you don't need the G_GNUC_PRINTF here, do you?

Indeed, it is incorrect.


r~