qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] libqtest: escape strings in QMP commands, fix leak
@ 2014-06-13  8:15 Paolo Bonzini
  2014-06-18  4:06 ` Stefan Hajnoczi
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Paolo Bonzini @ 2014-06-13  8:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: afaerber, stefanha

libqtest is using g_strdup_printf to format QMP commands, but
this does not work if the argument strings need to be escaped.
Instead, use the fancy %-formatting functionality of QObject.
The only change required in tests is that strings have to be
formatted as %s, not '%s' or \"%s\".  Luckily this usage of
parameterized QMP commands is not that frequent.

The leak is in socket_sendf.  Since we are extracting the send
loop to a new function, fix it now.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/fdc-test.c    |  2 +-
 tests/libqtest.c    | 47 +++++++++++++++++++++++++++++++++++++----------
 tests/qom-test.c    |  6 +++---
 tests/tmp105-test.c |  4 ++--
 4 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index 37096dc..c8e1e7b 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -291,7 +291,7 @@ static void test_media_insert(void)
     /* Insert media in drive. DSKCHK should not be reset until a step pulse
      * is sent. */
     qmp_discard_response("{'execute':'change', 'arguments':{"
-                         " 'device':'floppy0', 'target': '%s' }}",
+                         " 'device':'floppy0', 'target': %s }}",
                          test_image);
     qmp_discard_response(""); /* ignore event
                                  (FIXME open -> open transition?!) */
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 71468ac..98e8f4b 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -30,8 +30,9 @@
 
 #include "qemu/compiler.h"
 #include "qemu/osdep.h"
-#include "qapi/qmp/json-streamer.h"
 #include "qapi/qmp/json-parser.h"
+#include "qapi/qmp/json-streamer.h"
+#include "qapi/qmp/qjson.h"
 
 #define MAX_IRQ 256
 #define SOCKET_TIMEOUT 5
@@ -220,19 +221,15 @@ void qtest_quit(QTestState *s)
     g_free(s);
 }
 
-static void socket_sendf(int fd, const char *fmt, va_list ap)
+static void socket_send(int fd, const char *buf, size_t size)
 {
-    gchar *str;
-    size_t size, offset;
-
-    str = g_strdup_vprintf(fmt, ap);
-    size = strlen(str);
+    size_t offset;
 
     offset = 0;
     while (offset < size) {
         ssize_t len;
 
-        len = write(fd, str + offset, size - offset);
+        len = write(fd, buf + offset, size - offset);
         if (len == -1 && errno == EINTR) {
             continue;
         }
@@ -244,6 +241,15 @@ static void socket_sendf(int fd, const char *fmt, va_list ap)
     }
 }
 
+static void socket_sendf(int fd, const char *fmt, va_list ap)
+{
+    gchar *str = g_strdup_vprintf(fmt, ap);
+    size_t size = strlen(str);
+
+    socket_send(fd, str, size);
+    g_free(str);
+}
+
 static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
 {
     va_list ap;
@@ -378,8 +384,29 @@ QDict *qtest_qmp_receive(QTestState *s)
 
 QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
 {
-    /* Send QMP request */
-    socket_sendf(s->qmp_fd, fmt, ap);
+    va_list ap_copy;
+    QObject *qobj;
+
+    /* Going through qobject ensures we escape strings properly.
+     * This seemingly unnecessary copy is required in case va_list
+     * is an array type.
+     */
+    va_copy(ap_copy, ap);
+    qobj = qobject_from_jsonv(fmt, &ap_copy);
+    va_end(ap_copy);
+
+    /* No need to send anything for an empty QObject.  */
+    if (qobj) {
+        QString *qstr = qobject_to_json(qobj);
+        const char *str = qstring_get_str(qstr);
+        size_t size = qstring_get_length(qstr);
+
+        /* Send QMP request */
+        socket_send(s->qmp_fd, str, size);
+
+        QDECREF(qstr);
+        qobject_decref(qobj);
+    }
 
     /* Receive reply */
     return qtest_qmp_receive(s);
diff --git a/tests/qom-test.c b/tests/qom-test.c
index d8d1d8d..4246382 100644
--- a/tests/qom-test.c
+++ b/tests/qom-test.c
@@ -53,7 +53,7 @@ static void test_properties(const char *path, bool recurse)
 
     g_test_message("Obtaining properties of %s", path);
     response = qmp("{ 'execute': 'qom-list',"
-                   "  'arguments': { 'path': '%s' } }", path);
+                   "  'arguments': { 'path': %s } }", path);
     g_assert(response);
 
     if (!recurse) {
@@ -76,8 +76,8 @@ static void test_properties(const char *path, bool recurse)
             const char *prop = qdict_get_str(tuple, "name");
             g_test_message("Testing property %s.%s", path, prop);
             response = qmp("{ 'execute': 'qom-get',"
-                           "  'arguments': { 'path': '%s',"
-                           "                 'property': '%s' } }",
+                           "  'arguments': { 'path': %s,"
+                           "                 'property': %s } }",
                            path, prop);
             /* qom-get may fail but should not, e.g., segfault. */
             g_assert(response);
diff --git a/tests/tmp105-test.c b/tests/tmp105-test.c
index 15ddaf3..99db538 100644
--- a/tests/tmp105-test.c
+++ b/tests/tmp105-test.c
@@ -69,7 +69,7 @@ static int qmp_tmp105_get_temperature(const char *id)
     QDict *response;
     int ret;
 
-    response = qmp("{ 'execute': 'qom-get', 'arguments': { 'path': '%s', "
+    response = qmp("{ 'execute': 'qom-get', 'arguments': { 'path': %s, "
                    "'property': 'temperature' } }", id);
     g_assert(qdict_haskey(response, "return"));
     ret = qdict_get_int(response, "return");
@@ -81,7 +81,7 @@ static void qmp_tmp105_set_temperature(const char *id, int value)
 {
     QDict *response;
 
-    response = qmp("{ 'execute': 'qom-set', 'arguments': { 'path': '%s', "
+    response = qmp("{ 'execute': 'qom-set', 'arguments': { 'path': %s, "
                    "'property': 'temperature', 'value': %d } }", id, value);
     g_assert(qdict_haskey(response, "return"));
     QDECREF(response);
-- 
1.8.3.1

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

* Re: [Qemu-devel] [PATCH] libqtest: escape strings in QMP commands, fix leak
  2014-06-13  8:15 [Qemu-devel] [PATCH] libqtest: escape strings in QMP commands, fix leak Paolo Bonzini
@ 2014-06-18  4:06 ` Stefan Hajnoczi
  2014-06-18  7:41 ` Amos Kong
  2014-06-25  1:06 ` Amos Kong
  2 siblings, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2014-06-18  4:06 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, afaerber

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

On Fri, Jun 13, 2014 at 10:15:00AM +0200, Paolo Bonzini wrote:
> libqtest is using g_strdup_printf to format QMP commands, but
> this does not work if the argument strings need to be escaped.
> Instead, use the fancy %-formatting functionality of QObject.
> The only change required in tests is that strings have to be
> formatted as %s, not '%s' or \"%s\".  Luckily this usage of
> parameterized QMP commands is not that frequent.
> 
> The leak is in socket_sendf.  Since we are extracting the send
> loop to a new function, fix it now.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/fdc-test.c    |  2 +-
>  tests/libqtest.c    | 47 +++++++++++++++++++++++++++++++++++++----------
>  tests/qom-test.c    |  6 +++---
>  tests/tmp105-test.c |  4 ++--
>  4 files changed, 43 insertions(+), 16 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: [Qemu-devel] [PATCH] libqtest: escape strings in QMP commands, fix leak
  2014-06-13  8:15 [Qemu-devel] [PATCH] libqtest: escape strings in QMP commands, fix leak Paolo Bonzini
  2014-06-18  4:06 ` Stefan Hajnoczi
@ 2014-06-18  7:41 ` Amos Kong
  2014-06-18  9:56   ` Paolo Bonzini
  2014-06-25  1:06 ` Amos Kong
  2 siblings, 1 reply; 5+ messages in thread
From: Amos Kong @ 2014-06-18  7:41 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel, stefanha, afaerber

- Fixed Andreas's mail address

On Fri, Jun 13, 2014 at 10:15:00AM +0200, Paolo Bonzini wrote:
> libqtest is using g_strdup_printf to format QMP commands, but
> this does not work if the argument strings need to be escaped.
> Instead, use the fancy %-formatting functionality of QObject.
> The only change required in tests is that strings have to be
> formatted as %s, not '%s' or \"%s\".  Luckily this usage of
> parameterized QMP commands is not that frequent.

I got this error when I apply this patch (it works without this
patch):

  {"error": {"class": "GenericError", "desc": "Parameter 'id' expects an identifier"}}

Code:
|    QDict *response;
|    int i, j;
|
|    /* start with no network/block device, slots 3 to 0x1f are free */
|    qtest_start("-net none");
|
|    for (i = 3; i <= 0x1f; i++) {
|        for (j = 7; j >= 0; j--) {
|            response = qmp("{ 'execute': 'blockdev-add',"
|                           " 'arguments': {"
|                           "   'options': {"
|                           "     'driver': 'file',"
|                           "     'filename': '/dev/null',"
|                           "     'id': 'drv-%x.%x'"
                                         ^^^^^^^^^
|                           "} } }", i, j);
|            g_assert(response);
|            g_assert(!qdict_haskey(response, "error"));
|            QDECREF(response);



Then I have to fix it by :

     /* start with no network/block device, slots 3 to 0x1f are free */
     qtest_start("-net none");
 
     for (i = 3; i <= 0x1f; i++) {
         for (j = 7; j >= 0; j--) {
+            sprintf(drive_id, "drv-%x.%x", i, j);
             response = qmp("{ 'execute': 'blockdev-add',"
                            " 'arguments': {"
                            "   'options': {"
                            "     'driver': 'file',"
                            "     'filename': '/dev/null',"
-                           "     'id': 'drv-%x.%x'"
-                           "} } }", i, j);
+                           "     'id': %s"
+                           "} } }", drive_id);


Is it the expected result?


Thanks, Amos
 
> The leak is in socket_sendf.  Since we are extracting the send
> loop to a new function, fix it now.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/fdc-test.c    |  2 +-
>  tests/libqtest.c    | 47 +++++++++++++++++++++++++++++++++++++----------
>  tests/qom-test.c    |  6 +++---
>  tests/tmp105-test.c |  4 ++--
>  4 files changed, 43 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/fdc-test.c b/tests/fdc-test.c
> index 37096dc..c8e1e7b 100644
> --- a/tests/fdc-test.c
> +++ b/tests/fdc-test.c
> @@ -291,7 +291,7 @@ static void test_media_insert(void)
>      /* Insert media in drive. DSKCHK should not be reset until a step pulse
>       * is sent. */
>      qmp_discard_response("{'execute':'change', 'arguments':{"
> -                         " 'device':'floppy0', 'target': '%s' }}",
> +                         " 'device':'floppy0', 'target': %s }}",
>                           test_image);
>      qmp_discard_response(""); /* ignore event
>                                   (FIXME open -> open transition?!) */
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 71468ac..98e8f4b 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -30,8 +30,9 @@
>  
>  #include "qemu/compiler.h"
>  #include "qemu/osdep.h"
> -#include "qapi/qmp/json-streamer.h"
>  #include "qapi/qmp/json-parser.h"
> +#include "qapi/qmp/json-streamer.h"
> +#include "qapi/qmp/qjson.h"
>  
>  #define MAX_IRQ 256
>  #define SOCKET_TIMEOUT 5
> @@ -220,19 +221,15 @@ void qtest_quit(QTestState *s)
>      g_free(s);
>  }
>  
> -static void socket_sendf(int fd, const char *fmt, va_list ap)
> +static void socket_send(int fd, const char *buf, size_t size)
>  {
> -    gchar *str;
> -    size_t size, offset;
> -
> -    str = g_strdup_vprintf(fmt, ap);
> -    size = strlen(str);
> +    size_t offset;
>  
>      offset = 0;
>      while (offset < size) {
>          ssize_t len;
>  
> -        len = write(fd, str + offset, size - offset);
> +        len = write(fd, buf + offset, size - offset);
>          if (len == -1 && errno == EINTR) {
>              continue;
>          }
> @@ -244,6 +241,15 @@ static void socket_sendf(int fd, const char *fmt, va_list ap)
>      }
>  }
>  
> +static void socket_sendf(int fd, const char *fmt, va_list ap)
> +{
> +    gchar *str = g_strdup_vprintf(fmt, ap);
> +    size_t size = strlen(str);
> +
> +    socket_send(fd, str, size);
> +    g_free(str);
> +}
> +
>  static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
>  {
>      va_list ap;
> @@ -378,8 +384,29 @@ QDict *qtest_qmp_receive(QTestState *s)
>  
>  QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
>  {
> -    /* Send QMP request */
> -    socket_sendf(s->qmp_fd, fmt, ap);
> +    va_list ap_copy;
> +    QObject *qobj;
> +
> +    /* Going through qobject ensures we escape strings properly.
> +     * This seemingly unnecessary copy is required in case va_list
> +     * is an array type.
> +     */
> +    va_copy(ap_copy, ap);
> +    qobj = qobject_from_jsonv(fmt, &ap_copy);
> +    va_end(ap_copy);
> +
> +    /* No need to send anything for an empty QObject.  */
> +    if (qobj) {
> +        QString *qstr = qobject_to_json(qobj);
> +        const char *str = qstring_get_str(qstr);
> +        size_t size = qstring_get_length(qstr);
> +
> +        /* Send QMP request */
> +        socket_send(s->qmp_fd, str, size);
> +
> +        QDECREF(qstr);
> +        qobject_decref(qobj);
> +    }
>  
>      /* Receive reply */
>      return qtest_qmp_receive(s);
> diff --git a/tests/qom-test.c b/tests/qom-test.c
> index d8d1d8d..4246382 100644
> --- a/tests/qom-test.c
> +++ b/tests/qom-test.c
> @@ -53,7 +53,7 @@ static void test_properties(const char *path, bool recurse)
>  
>      g_test_message("Obtaining properties of %s", path);
>      response = qmp("{ 'execute': 'qom-list',"
> -                   "  'arguments': { 'path': '%s' } }", path);
> +                   "  'arguments': { 'path': %s } }", path);
>      g_assert(response);
>  
>      if (!recurse) {
> @@ -76,8 +76,8 @@ static void test_properties(const char *path, bool recurse)
>              const char *prop = qdict_get_str(tuple, "name");
>              g_test_message("Testing property %s.%s", path, prop);
>              response = qmp("{ 'execute': 'qom-get',"
> -                           "  'arguments': { 'path': '%s',"
> -                           "                 'property': '%s' } }",
> +                           "  'arguments': { 'path': %s,"
> +                           "                 'property': %s } }",
>                             path, prop);
>              /* qom-get may fail but should not, e.g., segfault. */
>              g_assert(response);
> diff --git a/tests/tmp105-test.c b/tests/tmp105-test.c
> index 15ddaf3..99db538 100644
> --- a/tests/tmp105-test.c
> +++ b/tests/tmp105-test.c
> @@ -69,7 +69,7 @@ static int qmp_tmp105_get_temperature(const char *id)
>      QDict *response;
>      int ret;
>  
> -    response = qmp("{ 'execute': 'qom-get', 'arguments': { 'path': '%s', "
> +    response = qmp("{ 'execute': 'qom-get', 'arguments': { 'path': %s, "
>                     "'property': 'temperature' } }", id);
>      g_assert(qdict_haskey(response, "return"));
>      ret = qdict_get_int(response, "return");
> @@ -81,7 +81,7 @@ static void qmp_tmp105_set_temperature(const char *id, int value)
>  {
>      QDict *response;
>  
> -    response = qmp("{ 'execute': 'qom-set', 'arguments': { 'path': '%s', "
> +    response = qmp("{ 'execute': 'qom-set', 'arguments': { 'path': %s, "
>                     "'property': 'temperature', 'value': %d } }", id, value);
>      g_assert(qdict_haskey(response, "return"));
>      QDECREF(response);
> -- 
> 1.8.3.1
> 

-- 
			Amos.

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

* Re: [Qemu-devel] [PATCH] libqtest: escape strings in QMP commands, fix leak
  2014-06-18  7:41 ` Amos Kong
@ 2014-06-18  9:56   ` Paolo Bonzini
  0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2014-06-18  9:56 UTC (permalink / raw)
  To: Amos Kong; +Cc: qemu-devel, stefanha, afaerber

Il 18/06/2014 09:41, Amos Kong ha scritto:
> - Fixed Andreas's mail address
>
> On Fri, Jun 13, 2014 at 10:15:00AM +0200, Paolo Bonzini wrote:
>> libqtest is using g_strdup_printf to format QMP commands, but
>> this does not work if the argument strings need to be escaped.
>> Instead, use the fancy %-formatting functionality of QObject.
>> The only change required in tests is that strings have to be
>> formatted as %s, not '%s' or \"%s\".  Luckily this usage of
>> parameterized QMP commands is not that frequent.
>
> I got this error when I apply this patch (it works without this
> patch):
>
>   {"error": {"class": "GenericError", "desc": "Parameter 'id' expects an identifier"}}
>
> Code:
> |    QDict *response;
> |    int i, j;
> |
> |    /* start with no network/block device, slots 3 to 0x1f are free */
> |    qtest_start("-net none");
> |
> |    for (i = 3; i <= 0x1f; i++) {
> |        for (j = 7; j >= 0; j--) {
> |            response = qmp("{ 'execute': 'blockdev-add',"
> |                           " 'arguments': {"
> |                           "   'options': {"
> |                           "     'driver': 'file',"
> |                           "     'filename': '/dev/null',"
> |                           "     'id': 'drv-%x.%x'"
>                                          ^^^^^^^^^
> |                           "} } }", i, j);
> |            g_assert(response);
> |            g_assert(!qdict_haskey(response, "error"));
> |            QDECREF(response);
>
>
>
> Then I have to fix it by :
>
>      /* start with no network/block device, slots 3 to 0x1f are free */
>      qtest_start("-net none");
>
>      for (i = 3; i <= 0x1f; i++) {
>          for (j = 7; j >= 0; j--) {
> +            sprintf(drive_id, "drv-%x.%x", i, j);
>              response = qmp("{ 'execute': 'blockdev-add',"
>                             " 'arguments': {"
>                             "   'options': {"
>                             "     'driver': 'file',"
>                             "     'filename': '/dev/null',"
> -                           "     'id': 'drv-%x.%x'"
> -                           "} } }", i, j);
> +                           "     'id': %s"
> +                           "} } }", drive_id);
>
>
> Is it the expected result?
>
>
> Thanks, Amos

Thanks Amos.  This is the right fix.

Paolo

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

* Re: [Qemu-devel] [PATCH] libqtest: escape strings in QMP commands, fix leak
  2014-06-13  8:15 [Qemu-devel] [PATCH] libqtest: escape strings in QMP commands, fix leak Paolo Bonzini
  2014-06-18  4:06 ` Stefan Hajnoczi
  2014-06-18  7:41 ` Amos Kong
@ 2014-06-25  1:06 ` Amos Kong
  2 siblings, 0 replies; 5+ messages in thread
From: Amos Kong @ 2014-06-25  1:06 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: afaerber, qemu-devel, stefanha

On Fri, Jun 13, 2014 at 10:15:00AM +0200, Paolo Bonzini wrote:
> libqtest is using g_strdup_printf to format QMP commands, but
> this does not work if the argument strings need to be escaped.
> Instead, use the fancy %-formatting functionality of QObject.
> The only change required in tests is that strings have to be
> formatted as %s, not '%s' or \"%s\".  Luckily this usage of
> parameterized QMP commands is not that frequent.
> 
> The leak is in socket_sendf.  Since we are extracting the send
> loop to a new function, fix it now.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Amos Kong <akong@redhat.com>

> ---
>  tests/fdc-test.c    |  2 +-
>  tests/libqtest.c    | 47 +++++++++++++++++++++++++++++++++++++----------
>  tests/qom-test.c    |  6 +++---
>  tests/tmp105-test.c |  4 ++--
>  4 files changed, 43 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/fdc-test.c b/tests/fdc-test.c
> index 37096dc..c8e1e7b 100644
> --- a/tests/fdc-test.c
> +++ b/tests/fdc-test.c
> @@ -291,7 +291,7 @@ static void test_media_insert(void)
>      /* Insert media in drive. DSKCHK should not be reset until a step pulse
>       * is sent. */
>      qmp_discard_response("{'execute':'change', 'arguments':{"
> -                         " 'device':'floppy0', 'target': '%s' }}",
> +                         " 'device':'floppy0', 'target': %s }}",
>                           test_image);
>      qmp_discard_response(""); /* ignore event
>                                   (FIXME open -> open transition?!) */
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 71468ac..98e8f4b 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -30,8 +30,9 @@
>  
>  #include "qemu/compiler.h"
>  #include "qemu/osdep.h"
> -#include "qapi/qmp/json-streamer.h"
>  #include "qapi/qmp/json-parser.h"
> +#include "qapi/qmp/json-streamer.h"
> +#include "qapi/qmp/qjson.h"
>  
>  #define MAX_IRQ 256
>  #define SOCKET_TIMEOUT 5
> @@ -220,19 +221,15 @@ void qtest_quit(QTestState *s)
>      g_free(s);
>  }
>  
> -static void socket_sendf(int fd, const char *fmt, va_list ap)
> +static void socket_send(int fd, const char *buf, size_t size)
>  {
> -    gchar *str;
> -    size_t size, offset;
> -
> -    str = g_strdup_vprintf(fmt, ap);
> -    size = strlen(str);
> +    size_t offset;
>  
>      offset = 0;
>      while (offset < size) {
>          ssize_t len;
>  
> -        len = write(fd, str + offset, size - offset);
> +        len = write(fd, buf + offset, size - offset);
>          if (len == -1 && errno == EINTR) {
>              continue;
>          }
> @@ -244,6 +241,15 @@ static void socket_sendf(int fd, const char *fmt, va_list ap)
>      }
>  }
>  
> +static void socket_sendf(int fd, const char *fmt, va_list ap)
> +{
> +    gchar *str = g_strdup_vprintf(fmt, ap);
> +    size_t size = strlen(str);
> +
> +    socket_send(fd, str, size);
> +    g_free(str);
> +}
> +
>  static void GCC_FMT_ATTR(2, 3) qtest_sendf(QTestState *s, const char *fmt, ...)
>  {
>      va_list ap;
> @@ -378,8 +384,29 @@ QDict *qtest_qmp_receive(QTestState *s)
>  
>  QDict *qtest_qmpv(QTestState *s, const char *fmt, va_list ap)
>  {
> -    /* Send QMP request */
> -    socket_sendf(s->qmp_fd, fmt, ap);
> +    va_list ap_copy;
> +    QObject *qobj;
> +
> +    /* Going through qobject ensures we escape strings properly.
> +     * This seemingly unnecessary copy is required in case va_list
> +     * is an array type.
> +     */
> +    va_copy(ap_copy, ap);
> +    qobj = qobject_from_jsonv(fmt, &ap_copy);
> +    va_end(ap_copy);
> +
> +    /* No need to send anything for an empty QObject.  */
> +    if (qobj) {
> +        QString *qstr = qobject_to_json(qobj);
> +        const char *str = qstring_get_str(qstr);
> +        size_t size = qstring_get_length(qstr);
> +
> +        /* Send QMP request */
> +        socket_send(s->qmp_fd, str, size);
> +
> +        QDECREF(qstr);
> +        qobject_decref(qobj);
> +    }
>  
>      /* Receive reply */
>      return qtest_qmp_receive(s);
> diff --git a/tests/qom-test.c b/tests/qom-test.c
> index d8d1d8d..4246382 100644
> --- a/tests/qom-test.c
> +++ b/tests/qom-test.c
> @@ -53,7 +53,7 @@ static void test_properties(const char *path, bool recurse)
>  
>      g_test_message("Obtaining properties of %s", path);
>      response = qmp("{ 'execute': 'qom-list',"
> -                   "  'arguments': { 'path': '%s' } }", path);
> +                   "  'arguments': { 'path': %s } }", path);
>      g_assert(response);
>  
>      if (!recurse) {
> @@ -76,8 +76,8 @@ static void test_properties(const char *path, bool recurse)
>              const char *prop = qdict_get_str(tuple, "name");
>              g_test_message("Testing property %s.%s", path, prop);
>              response = qmp("{ 'execute': 'qom-get',"
> -                           "  'arguments': { 'path': '%s',"
> -                           "                 'property': '%s' } }",
> +                           "  'arguments': { 'path': %s,"
> +                           "                 'property': %s } }",
>                             path, prop);
>              /* qom-get may fail but should not, e.g., segfault. */
>              g_assert(response);
> diff --git a/tests/tmp105-test.c b/tests/tmp105-test.c
> index 15ddaf3..99db538 100644
> --- a/tests/tmp105-test.c
> +++ b/tests/tmp105-test.c
> @@ -69,7 +69,7 @@ static int qmp_tmp105_get_temperature(const char *id)
>      QDict *response;
>      int ret;
>  
> -    response = qmp("{ 'execute': 'qom-get', 'arguments': { 'path': '%s', "
> +    response = qmp("{ 'execute': 'qom-get', 'arguments': { 'path': %s, "
>                     "'property': 'temperature' } }", id);
>      g_assert(qdict_haskey(response, "return"));
>      ret = qdict_get_int(response, "return");
> @@ -81,7 +81,7 @@ static void qmp_tmp105_set_temperature(const char *id, int value)
>  {
>      QDict *response;
>  
> -    response = qmp("{ 'execute': 'qom-set', 'arguments': { 'path': '%s', "
> +    response = qmp("{ 'execute': 'qom-set', 'arguments': { 'path': %s, "
>                     "'property': 'temperature', 'value': %d } }", id, value);
>      g_assert(qdict_haskey(response, "return"));
>      QDECREF(response);
> -- 
> 1.8.3.1
> 

-- 
			Amos.

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

end of thread, other threads:[~2014-06-25  1:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-13  8:15 [Qemu-devel] [PATCH] libqtest: escape strings in QMP commands, fix leak Paolo Bonzini
2014-06-18  4:06 ` Stefan Hajnoczi
2014-06-18  7:41 ` Amos Kong
2014-06-18  9:56   ` Paolo Bonzini
2014-06-25  1:06 ` Amos Kong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).