[PULL 0/4] docs + qtest patches

[PULL 0/4] docs + qtest patches

[Thomas Huth]

 Hi Peter,

the following changes since commit 7fbd7e710323c8f4c5f6a38a8ae0e6726b5a4599:

  Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/tracing-pull-request' into staging (2020-11-19 17:08:42 +0000)

are available in the Git repository at:

  https://gitlab.com/huth/qemu.git tags/pull-request-2020-11-20

for you to fetch changes up to 88a8b27e556f5cb49a19d4c98a96590f61b74ed2:

  qtest: do not return freed argument vector from qtest_rsp (2020-11-20 15:04:16 +0100)

gitlab-CI run: https://gitlab.com/huth/qemu/-/pipelines/219170774

----------------------------------------------------------------
* Some small qtest fixes
* One documentation update
----------------------------------------------------------------

Chen Qun (2):
      tests/qtest: variable defined by g_autofree need to be initialized
      tests/qtest: fix memleak in npcm7xx_watchdog_timer-test

Paolo Bonzini (1):
      qtest: do not return freed argument vector from qtest_rsp

Thomas Huth (1):
      docs: Get rid of the weird _005f links in the man page

 docs/system/gdb.rst                       |  2 +-
 docs/system/images.rst                    |  2 +-
 docs/system/monitor.rst                   |  2 +-
 docs/system/mux-chardev.rst               |  2 +-
 docs/system/usb.rst                       |  6 ++---
 docs/system/vnc-security.rst              |  2 +-
 qemu-options.hx                           | 38 ++++++++++++++++---------------
 tests/qtest/libqtest.c                    |  1 +
 tests/qtest/npcm7xx_timer-test.c          |  8 +++----
 tests/qtest/npcm7xx_watchdog_timer-test.c |  6 +++--
 10 files changed, 36 insertions(+), 33 deletions(-)

[PULL 1/4] docs: Get rid of the weird _005f links in the man page

[Thomas Huth]

The man page does not contain all the chapters from the System Emulation
Users Guide, so some of the links that we've put into the qemu options
descriptions can not be resolved and thus the link names are used in the
man pages instead. These link names currently contain weird "_005f" letters
in the middle and just do not make any sense for the users. To avoid this
situation, replace the link names with more descriptive, natural text.

Message-Id: <20201116145341.91606-1-thuth@redhat.com>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3
Buglink: https://bugs.launchpad.net/qemu/+bug/1453608
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/system/gdb.rst          |  2 +-
 docs/system/images.rst       |  2 +-
 docs/system/monitor.rst      |  2 +-
 docs/system/mux-chardev.rst  |  2 +-
 docs/system/usb.rst          |  6 +++---
 docs/system/vnc-security.rst |  2 +-
 qemu-options.hx              | 38 +++++++++++++++++++-----------------
 7 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/docs/system/gdb.rst b/docs/system/gdb.rst
index abda961e2b..72b1e68f4e 100644
--- a/docs/system/gdb.rst
+++ b/docs/system/gdb.rst
@@ -1,4 +1,4 @@
-.. _gdb_005fusage:
+.. _GDB usage:
 
 GDB usage
 ---------
diff --git a/docs/system/images.rst b/docs/system/images.rst
index ff26bf9587..3d9144e625 100644
--- a/docs/system/images.rst
+++ b/docs/system/images.rst
@@ -1,4 +1,4 @@
-.. _disk_005fimages:
+.. _disk images:
 
 Disk Images
 -----------
diff --git a/docs/system/monitor.rst b/docs/system/monitor.rst
index 0bcd5da216..ff5c434612 100644
--- a/docs/system/monitor.rst
+++ b/docs/system/monitor.rst
@@ -1,4 +1,4 @@
-.. _pcsys_005fmonitor:
+.. _QEMU monitor:
 
 QEMU Monitor
 ------------
diff --git a/docs/system/mux-chardev.rst b/docs/system/mux-chardev.rst
index 413a6b3446..05064068aa 100644
--- a/docs/system/mux-chardev.rst
+++ b/docs/system/mux-chardev.rst
@@ -1,4 +1,4 @@
-.. _mux_005fkeys:
+.. _keys in the character backend multiplexer:
 
 Keys in the character backend multiplexer
 -----------------------------------------
diff --git a/docs/system/usb.rst b/docs/system/usb.rst
index 9a2f1927c4..eeab78dcfb 100644
--- a/docs/system/usb.rst
+++ b/docs/system/usb.rst
@@ -8,7 +8,7 @@ plug virtual USB devices or real host USB devices (only works with
 certain host operating systems). QEMU will automatically create and
 connect virtual USB hubs as necessary to connect multiple USB devices.
 
-.. _usb_005fdevices:
+.. _Connecting USB devices:
 
 Connecting USB devices
 ~~~~~~~~~~~~~~~~~~~~~~
@@ -27,8 +27,8 @@ option or the ``device_add`` monitor command. Available devices are:
    activated.
 
 ``usb-storage,drive=drive_id``
-   Mass storage device backed by drive_id (see
-   :ref:`disk_005fimages`)
+   Mass storage device backed by drive_id (see the :ref:`disk images`
+   chapter in the System Emulation Users Guide)
 
 ``usb-uas``
    USB attached SCSI device, see
diff --git a/docs/system/vnc-security.rst b/docs/system/vnc-security.rst
index b237b07330..558e4faffc 100644
--- a/docs/system/vnc-security.rst
+++ b/docs/system/vnc-security.rst
@@ -1,4 +1,4 @@
-.. _vnc_005fsecurity:
+.. _VNC security:
 
 VNC security
 ------------
diff --git a/qemu-options.hx b/qemu-options.hx
index 2c83390504..104632ea34 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -913,8 +913,8 @@ SRST
 ``-fda file``
   \
 ``-fdb file``
-    Use file as floppy disk 0/1 image (see
-    :ref:`disk_005fimages`).
+    Use file as floppy disk 0/1 image (see the :ref:`disk images` chapter in
+    the System Emulation Users Guide).
 ERST
 
 DEF("hda", HAS_ARG, QEMU_OPTION_hda,
@@ -931,8 +931,8 @@ SRST
 ``-hdc file``
   \ 
 ``-hdd file``
-    Use file as hard disk 0, 1, 2 or 3 image (see
-    :ref:`disk_005fimages`).
+    Use file as hard disk 0, 1, 2 or 3 image (see the :ref:`disk images`
+    chapter in the System Emulation Users Guide).
 ERST
 
 DEF("cdrom", HAS_ARG, QEMU_OPTION_cdrom,
@@ -1189,9 +1189,9 @@ SRST
     In addition, it knows the following options:
 
     ``file=file``
-        This option defines which disk image (see
-        :ref:`disk_005fimages`) to use with this drive. If
-        the filename contains comma, you must double it (for instance,
+        This option defines which disk image (see the :ref:`disk images`
+        chapter in the System Emulation Users Guide) to use with this drive.
+        If the filename contains comma, you must double it (for instance,
         "file=my,,file" to use file "my,file").
 
         Special files such as iSCSI devices can be specified using
@@ -1400,8 +1400,8 @@ SRST
 ``-snapshot``
     Write to temporary files instead of disk image files. In this case,
     the raw disk image you use is not written back. You can however
-    force the write back by pressing C-a s (see
-    :ref:`disk_005fimages`).
+    force the write back by pressing C-a s (see the :ref:`disk images`
+    chapter in the System Emulation Users Guide).
 ERST
 
 DEF("fsdev", HAS_ARG, QEMU_OPTION_fsdev,
@@ -1687,8 +1687,8 @@ DEF("usbdevice", HAS_ARG, QEMU_OPTION_usbdevice,
 SRST
 ``-usbdevice devname``
     Add the USB device devname. Note that this option is deprecated,
-    please use ``-device usb-...`` instead. See
-    :ref:`usb_005fdevices`.
+    please use ``-device usb-...`` instead. See the chapter about
+    :ref:`Connecting USB devices` in the System Emulation Users Guide.
 
     ``mouse``
         Virtual Mouse. This will override the PS/2 mouse emulation when
@@ -2110,7 +2110,7 @@ SRST
         connections.
 
         The password must be set separately using the ``set_password``
-        command in the :ref:`pcsys_005fmonitor`. The
+        command in the :ref:`QEMU monitor`. The
         syntax to change your password is:
         ``set_password <protocol> <password>`` where <protocol> could be
         either "vnc" or "spice".
@@ -2155,8 +2155,8 @@ SRST
         and 'x509' settings to enable use of SSL and server
         certificates. This ensures a data encryption preventing
         compromise of authentication credentials. See the
-        :ref:`vnc_005fsecurity` section for details on
-        using SASL authentication.
+        :ref:`VNC security` section in the System Emulation Users Guide
+        for details on using SASL authentication.
 
     ``sasl-authz=ID``
         Provides the ID of the QAuthZ authorization object against which
@@ -3082,7 +3082,9 @@ The general form of a character device option is:
         -serial chardev:char1
 
     When you're using a multiplexed character device, some escape
-    sequences are interpreted in the input. See :ref:`mux_005fkeys`.
+    sequences are interpreted in the input. See the chapter about
+    :ref:`keys in the character backend multiplexer` in the
+    System Emulation Users Guide for more details.
 
     Note that some other command line options may implicitly create
     multiplexed character backends; for instance ``-serial mon:stdio``
@@ -3755,8 +3757,8 @@ DEF("gdb", HAS_ARG, QEMU_OPTION_gdb, \
     QEMU_ARCH_ALL)
 SRST
 ``-gdb dev``
-    Accept a gdb connection on device dev (see
-    :ref:`gdb_005fusage`). Note that this option does not pause QEMU
+    Accept a gdb connection on device dev (see the :ref:`GDB usage` chapter
+    in the System Emulation Users Guide). Note that this option does not pause QEMU
     execution -- if you want QEMU to not start the guest until you
     connect with gdb and issue a ``continue`` command, you will need to
     also pass the ``-S`` option to QEMU.
@@ -3781,7 +3783,7 @@ DEF("s", 0, QEMU_OPTION_s, \
 SRST
 ``-s``
     Shorthand for -gdb tcp::1234, i.e. open a gdbserver on TCP port 1234
-    (see :ref:`gdb_005fusage`).
+    (see the :ref:`GDB usage` chapter in the System Emulation Users Guide).
 ERST
 
 DEF("d", HAS_ARG, QEMU_OPTION_d, \
-- 
2.18.4

[PULL 2/4] tests/qtest: variable defined by g_autofree need to be initialized

[Thomas Huth]

From: Chen Qun <kuhn.chenqun@huawei.com>

According to the glib function requirements, we need initialise
 the variable. Otherwise there will be compilation warnings:

glib-autocleanups.h:28:3: warning: ‘full_name’ may be
used uninitialized in this function [-Wmaybe-uninitialized]
   28 |   g_free (*pp);
      |   ^~~~~~~~~~~~

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Havard Skinnemoen <hskinnemoen@google.com>
Message-Id: <20201118115646.2461726-2-kuhn.chenqun@huawei.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/npcm7xx_timer-test.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/npcm7xx_timer-test.c b/tests/qtest/npcm7xx_timer-test.c
index f08b0cd62a..83774a5b90 100644
--- a/tests/qtest/npcm7xx_timer-test.c
+++ b/tests/qtest/npcm7xx_timer-test.c
@@ -512,11 +512,9 @@ static void test_disable_on_expiration(gconstpointer test_data)
  */
 static void tim_add_test(const char *name, const TestData *td, GTestDataFunc fn)
 {
-    g_autofree char *full_name;
-
-    full_name = g_strdup_printf("npcm7xx_timer/tim[%d]/timer[%d]/%s",
-                                tim_index(td->tim), timer_index(td->timer),
-                                name);
+    g_autofree char *full_name = g_strdup_printf(
+        "npcm7xx_timer/tim[%d]/timer[%d]/%s", tim_index(td->tim),
+        timer_index(td->timer), name);
     qtest_add_data_func(full_name, td, fn);
 }
 
-- 
2.18.4

[PULL 3/4] tests/qtest: fix memleak in npcm7xx_watchdog_timer-test

[Thomas Huth]

From: Chen Qun <kuhn.chenqun@huawei.com>

Properly free resp for get_watchdog_action() to avoid memory leak.
ASAN shows memory leak stack:

Indirect leak of 12360 byte(s) in 3 object(s) allocated from:
    #0 0x7f41ab6cbd4e in __interceptor_calloc (/lib64/libasan.so.5+0x112d4e)
    #1 0x7f41ab4eaa50 in g_malloc0 (/lib64/libglib-2.0.so.0+0x55a50)
    #2 0x556487d5374b in qdict_new ../qobject/qdict.c:29
    #3 0x556487d65e1a in parse_object ../qobject/json-parser.c:318
    #4 0x556487d65cb6 in parse_pair ../qobject/json-parser.c:287
    #5 0x556487d65ebd in parse_object ../qobject/json-parser.c:343
    #6 0x556487d661d5 in json_parser_parse ../qobject/json-parser.c:580
    #7 0x556487d513df in json_message_process_token ../qobject/json-streamer.c:92
    #8 0x556487d63919 in json_lexer_feed_char ../qobject/json-lexer.c:313
    #9 0x556487d63d75 in json_lexer_feed ../qobject/json-lexer.c:350
    #10 0x556487d28b2a in qmp_fd_receive ../tests/qtest/libqtest.c:613
    #11 0x556487d2a16f in qtest_qmp_eventwait_ref ../tests/qtest/libqtest.c:827
    #12 0x556487d248e2 in get_watchdog_action ../tests/qtest/npcm7xx_watchdog_timer-test.c:94
    #13 0x556487d25765 in test_enabling_flags ../tests/qtest/npcm7xx_watchdog_timer-test.c:243

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
Message-Id: <20201118115646.2461726-3-kuhn.chenqun@huawei.com>
Reviewed-by: Havard Skinnemoen <hskinnemoen@google.com>
Reviewed-by: Hao Wu <wuhaotsh@google.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/npcm7xx_watchdog_timer-test.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/npcm7xx_watchdog_timer-test.c b/tests/qtest/npcm7xx_watchdog_timer-test.c
index 54d5d6d8f2..3aae5a0438 100644
--- a/tests/qtest/npcm7xx_watchdog_timer-test.c
+++ b/tests/qtest/npcm7xx_watchdog_timer-test.c
@@ -204,6 +204,7 @@ static void test_enabling_flags(gconstpointer watchdog)
 {
     const Watchdog *wd = watchdog;
     QTestState *qts;
+    QDict *rsp;
 
     /* Neither WTIE or WTRE is set, no interrupt or reset should happen */
     qts = qtest_init("-machine quanta-gsj");
@@ -240,8 +241,9 @@ static void test_enabling_flags(gconstpointer watchdog)
     g_assert_false(qtest_get_irq(qts, wd->irq));
     qtest_clock_step(qts, watchdog_calculate_steps(RESET_CYCLES,
                 watchdog_prescaler(qts, wd)));
-    g_assert_false(strcmp(qdict_get_str(get_watchdog_action(qts), "action"),
-                "reset"));
+    rsp = get_watchdog_action(qts);
+    g_assert_false(strcmp(qdict_get_str(rsp, "action"), "reset"));
+    qobject_unref(rsp);
     qtest_qmp_eventwait(qts, "RESET");
     qtest_quit(qts);
 
-- 
2.18.4

[PULL 4/4] qtest: do not return freed argument vector from qtest_rsp

[Thomas Huth]

From: Paolo Bonzini <pbonzini@redhat.com>

If expected_args is 0, qtest frees the argument vector and then returns it
nevertheless.  Coverity complains; in practice this is not an issue because
expected_args == 0 means that the caller is not interested in the argument
vector, but it would be a potential problem if somebody wanted to add
commands with optional arguments to qtest.

Suggested-by: Kamil Dudka <kdudka@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201120073149.99079-1-pbonzini@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/libqtest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index be0fb430dd..e49f3a1e45 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -545,6 +545,7 @@ redo:
         }
     } else {
         g_strfreev(words);
+        words = NULL;
     }
 
     return words;
-- 
2.18.4

Re: [PULL 0/4] docs + qtest patches

[Peter Maydell]

On Fri, 20 Nov 2020 at 17:18, Thomas Huth <thuth@redhat.com> wrote:
>
>  Hi Peter,
>
> the following changes since commit 7fbd7e710323c8f4c5f6a38a8ae0e6726b5a4599:
>
>   Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/tracing-pull-request' into staging (2020-11-19 17:08:42 +0000)
>
> are available in the Git repository at:
>
>   https://gitlab.com/huth/qemu.git tags/pull-request-2020-11-20
>
> for you to fetch changes up to 88a8b27e556f5cb49a19d4c98a96590f61b74ed2:
>
>   qtest: do not return freed argument vector from qtest_rsp (2020-11-20 15:04:16 +0100)
>
> gitlab-CI run: https://gitlab.com/huth/qemu/-/pipelines/219170774
>
> ----------------------------------------------------------------
> * Some small qtest fixes
> * One documentation update



Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.

-- PMM