[Qemu-devel] [PATCH] libqtest: handle zero length memwrite/memread

[Qemu-devel] [PATCH] libqtest: handle zero length memwrite/memread

[Greg Kurz]

Some recently added tests pass a zero length to qtest_memwrite().
Unfortunately, the qtest protocol doesn't implement an on-the-wire
syntax for zero-length writes and the current code happily sends
garbage to QEMU. This causes intermittent failures.

It isn't worth the pain to enhance the protocol, so this patch
simply fixes the issue by "just return, doing nothing". The same
fix is applied to qtest_memread() since the issue also exists in
the QEMU part of the "memread" command.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 qtest.c          |    2 ++
 tests/libqtest.c |   12 +++++++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/qtest.c b/qtest.c
index 46b99aed5291..bd9d4178129b 100644
--- a/qtest.c
+++ b/qtest.c
@@ -430,6 +430,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         g_assert(words[1] && words[2]);
         g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
         g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
+        /* We'd send garbage to libqtest if len is 0 */
+        g_assert(len);
 
         data = g_malloc(len);
         cpu_physical_memory_read(addr, data, len);
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 6f6975248fae..d8fba6647a17 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -768,6 +768,10 @@ void qtest_memread(QTestState *s, uint64_t addr, void *data, size_t size)
     gchar **args;
     size_t i;
 
+    if (!size) {
+        return;
+    }
+
     qtest_sendf(s, "read 0x%" PRIx64 " 0x%zx\n", addr, size);
     args = qtest_rsp(s, 2);
 
@@ -858,7 +862,13 @@ void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
 {
     const uint8_t *ptr = data;
     size_t i;
-    char *enc = g_malloc(2 * size + 1);
+    char *enc;
+
+    if (!size) {
+        return;
+    }
+
+    enc = g_malloc(2 * size + 1);
 
     for (i = 0; i < size; i++) {
         sprintf(&enc[i * 2], "%02x", ptr[i]);

Re: [Qemu-devel] [PATCH] libqtest: handle zero length memwrite/memread

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

On 01/11/2017 02:49 AM, Greg Kurz wrote:
> Some recently added tests pass a zero length to qtest_memwrite().
> Unfortunately, the qtest protocol doesn't implement an on-the-wire
> syntax for zero-length writes and the current code happily sends
> garbage to QEMU. This causes intermittent failures.
> 
> It isn't worth the pain to enhance the protocol, so this patch
> simply fixes the issue by "just return, doing nothing". The same
> fix is applied to qtest_memread() since the issue also exists in
> the QEMU part of the "memread" command.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  qtest.c          |    2 ++
>  tests/libqtest.c |   12 +++++++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] libqtest: handle zero length memwrite/memread

[John Snow]

On 01/11/2017 03:49 AM, Greg Kurz wrote:
> Some recently added tests pass a zero length to qtest_memwrite().
> Unfortunately, the qtest protocol doesn't implement an on-the-wire
> syntax for zero-length writes and the current code happily sends
> garbage to QEMU. This causes intermittent failures.
> 
> It isn't worth the pain to enhance the protocol, so this patch
> simply fixes the issue by "just return, doing nothing". The same
> fix is applied to qtest_memread() since the issue also exists in
> the QEMU part of the "memread" command.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  qtest.c          |    2 ++
>  tests/libqtest.c |   12 +++++++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/qtest.c b/qtest.c
> index 46b99aed5291..bd9d4178129b 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -430,6 +430,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
>          g_assert(words[1] && words[2]);
>          g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
>          g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
> +        /* We'd send garbage to libqtest if len is 0 */
> +        g_assert(len);
>  
>          data = g_malloc(len);
>          cpu_physical_memory_read(addr, data, len);
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 6f6975248fae..d8fba6647a17 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -768,6 +768,10 @@ void qtest_memread(QTestState *s, uint64_t addr, void *data, size_t size)
>      gchar **args;
>      size_t i;
>  
> +    if (!size) {
> +        return;
> +    }
> +
>      qtest_sendf(s, "read 0x%" PRIx64 " 0x%zx\n", addr, size);
>      args = qtest_rsp(s, 2);
>  
> @@ -858,7 +862,13 @@ void qtest_memwrite(QTestState *s, uint64_t addr, const void *data, size_t size)
>  {
>      const uint8_t *ptr = data;
>      size_t i;
> -    char *enc = g_malloc(2 * size + 1);
> +    char *enc;
> +
> +    if (!size) {
> +        return;
> +    }
> +
> +    enc = g_malloc(2 * size + 1);
>  
>      for (i = 0; i < size; i++) {
>          sprintf(&enc[i * 2], "%02x", ptr[i]);
> 
> 

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [PATCH] libqtest: handle zero length memwrite/memread

[Peter Maydell]

On 11 January 2017 at 08:49, Greg Kurz <groug@kaod.org> wrote:
> Some recently added tests pass a zero length to qtest_memwrite().
> Unfortunately, the qtest protocol doesn't implement an on-the-wire
> syntax for zero-length writes and the current code happily sends
> garbage to QEMU. This causes intermittent failures.
>
> It isn't worth the pain to enhance the protocol, so this patch
> simply fixes the issue by "just return, doing nothing". The same
> fix is applied to qtest_memread() since the issue also exists in
> the QEMU part of the "memread" command.
>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---

Thanks; applied to master since it should fix some
intermittent failures in the tests I run on new merges.

-- PMM