From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:36822)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1WxCc6-0002AX-5G
	for qemu-devel@nongnu.org; Wed, 18 Jun 2014 05:57:12 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1WxCby-000200-M0
	for qemu-devel@nongnu.org; Wed, 18 Jun 2014 05:57:06 -0400
Received: from mx1.redhat.com ([209.132.183.28]:13562)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <pbonzini@redhat.com>) id 1WxCby-0001zZ-EN
	for qemu-devel@nongnu.org; Wed, 18 Jun 2014 05:56:58 -0400
Message-ID: <53A16265.9010500@redhat.com>
Date: Wed, 18 Jun 2014 11:56:53 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
References: <1402647300-27861-1-git-send-email-pbonzini@redhat.com>
	<20140618074158.GD18929@z.redhat.com>
In-Reply-To: <20140618074158.GD18929@z.redhat.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] libqtest: escape strings in QMP commands,
 fix leak
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Amos Kong <akong@redhat.com>
Cc: qemu-devel@nongnu.org, stefanha@redhat.com, afaerber@suse.de

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