Re: [Qemu-devel] [PATCH] libqtest: escape strings in QMP commands, fix leak

[Paolo Bonzini <pbonzini@redhat.com>]

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