Re: [Qemu-devel] [PATCH 3/3] tests: more strict command batching test

[Peter Xu]

On Wed, Mar 21, 2018 at 07:55:19AM -0500, Eric Blake wrote:
> On 03/21/2018 01:55 AM, Peter Xu wrote:
> > Add "id" fields to the commands, and check that the command returns are
> > in order.
> > 
> > Signed-off-by: Peter Xu <address@hidden>
> > ---
> >   tests/qmp-test.c | 8 +++++++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> 
> > +++ b/tests/qmp-test.c
> > @@ -83,6 +83,7 @@ static void test_qmp_protocol(void)
> >       const QListEntry *entry;
> >       QString *qstr;
> >       int i;
> > +    char buf[128];
> 
> Eww, we shouldn't need this.
> 
> >       qts = qtest_init_without_qmp_handshake(common_args);
> > @@ -150,7 +151,10 @@ static void test_qmp_protocol(void)
> >        * best-effort test.
> >        */
> >       for (i = 0; i < 16; i++) {
> > -        qtest_async_qmp(qts, "{ 'execute': 'query-version' }");
> > +        snprintf(buf, sizeof(buf) - 1, "{ 'execute': 'query-version', "
> > +                 "'id': %d }", i);
> > +        buf[sizeof(buf) - 1] = '\0';
> > +        qtest_async_qmp(qts, buf);
> 
> snprintf() of JSON strings is prone to failures especially when the JSON
> string is reinterpreted as a printf argument in qtest_async_qmp.  Better is
> to let qtest_async_qmp() directly do the formatting, as in:
> 
> qtest_async_qmp(qts, "{ 'execute':'query-version', 'id':%d}", i);
> 
> And then I was right - you don't need buf.

Ouch. :)

Will fix that. Thanks,

> 
> Otherwise the addition is good.
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org

-- 
Peter Xu