All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: qemu-devel@nongnu.org, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH v4 05/22] qobject: Simplify qobject_from_jsonv()
Date: Mon, 02 Oct 2017 07:46:47 +0200	[thread overview]
Message-ID: <87lgkurtiw.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <c9564021-896a-2c29-8f74-0921e94d292a@redhat.com> (Eric Blake's message of "Wed, 9 Aug 2017 08:14:09 -0500")

Eric Blake <eblake@redhat.com> writes:

> On 08/09/2017 02:59 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>> 
>>> qobject_from_jsonv() was unusual in that it took a va_list*, instead
>>> of the more typical va_list; this was so that callers could pass
>>> NULL to avoid % interpolation.  While this works under the hood, it
>>> is awkward for callers, so move the magic into qjson.c rather than
>>> in the public interface, and finally improve the documentation of
>>> qobject_from_jsonf().
>>>
>
>>> -    /* 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, &error_abort);
>>> -    va_end(ap_copy);
>>> +    /* Going through qobject ensures we escape strings properly. */
>>> +    qobj = qobject_from_jsonv(fmt, ap);
>>>      qstr = qobject_to_json(qobj);
>>>
>>>      /*
>> 
>> Wait!  Oh, the va_copy() moves iinto qobject_from_jsonv().  Okay, I
>> guess.
[...]
>>> +
>>> +    /*
>>> +     * This seemingly unnecessary copy is required in case va_list
>>> +     * is an array type.
>>> +     */
>> 
>> --verbose?
>>
>>> +    va_copy(ap_copy, ap);
>>> +    obj = qobject_from_json_internal(string, &ap_copy, &error_abort);
>>> +    va_end(ap_copy);
>
> Code motion. But if the comment needs to be more verbose in the
> destination than it was on the source, the rationale is that C99/POSIX
> allows 'typedef something va_list[]' (that is, where va_list is an array
> of some other type), although I don't know of any modern OS that
> actually defines it like that.  Based on C pointer-decay rules, '&ap'
> has a different type based on whether va_list was a struct/pointer or an
> array type, when 'va_list ap' was passed as a parameter; so we can't
> portably use qobject_from_json_internal(string, &ap, &error_abort).  The
> va_copy() is what lets us guarantee that &ap_list is a pointer to a
> va_list regardless of the type of va_list (because va_copy was declared
> locally, rather than in a parameter list, and is therefore not subject
> to pointer decay), and NOT an accidental pointer to first element of the
> va_list array on platforms where va_list is an array.

I'm dense this Monday morning --- I still can't see where exactly
passing &ap directly goes wrong.

Two cases:

1. va_list is a typedef name for a non-array type T.

2. va_list is a typedef name for an array type E[].

What are the types of actual argument &ap and formal parameter va_list
*ap in either case?

How exactly does case 2 break?

  reply	other threads:[~2017-10-02  5:46 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-04  1:25 [Qemu-devel] [PATCH v4 00/22] Clean up around qmp() and hmp() Eric Blake
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 01/22] qobject: Accept "%"PRId64 in qobject_from_jsonf() Eric Blake
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 02/22] tests: Clean up wait for event Eric Blake
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 03/22] tests/libqtest: Clean up how we read the QMP greeting Eric Blake
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 04/22] tests: Add assertion for no qmp("") Eric Blake
2017-08-09  7:13   ` Markus Armbruster
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 05/22] qobject: Simplify qobject_from_jsonv() Eric Blake
2017-08-09  7:59   ` Markus Armbruster
2017-08-09 13:14     ` Eric Blake
2017-10-02  5:46       ` Markus Armbruster [this message]
2017-10-02 14:30         ` Eric Blake
2018-01-08 16:46           ` Eric Blake
2017-09-11 21:52   ` Eric Blake
2017-10-02  7:15     ` Markus Armbruster
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 06/22] qobject: Perform %% interpolation in qobject_from_jsonf() Eric Blake
2017-08-09  9:06   ` Markus Armbruster
2017-08-09 13:21     ` Eric Blake
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 07/22] numa-test: Use hmp() Eric Blake
2017-08-09  9:07   ` Markus Armbruster
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 08/22] qtest: Avoid passing raw strings through hmp() Eric Blake
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 09/22] qtest: Document calling conventions Eric Blake
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 10/22] libqtest: Skip round-trip through QObject Eric Blake
2017-08-09 10:10   ` Markus Armbruster
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 11/22] test-qga: Simplify command construction Eric Blake
2017-08-09 11:40   ` Markus Armbruster
2017-08-09 13:29     ` Eric Blake
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 12/22] libqtest: Change qmp_fd_send() to drop varargs Eric Blake
2017-08-09 13:18   ` Markus Armbruster
2017-08-09 13:44     ` Eric Blake
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 13/22] libqtest: Add qmp_raw() Eric Blake
2017-08-09 14:54   ` Markus Armbruster
2017-08-09 15:18     ` Eric Blake
2017-08-10  7:29       ` Markus Armbruster
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 14/22] libqtest: Separate qmp_discard_response() from command Eric Blake
2017-08-09 15:15   ` Markus Armbruster
2017-08-09 15:32     ` Eric Blake
2017-08-10  7:40       ` Markus Armbruster
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 15/22] libqtest: Delete qtest_qmp() wrappers Eric Blake
2017-08-09 15:34   ` Markus Armbruster
2017-08-09 16:35     ` Eric Blake
2017-08-10  7:47       ` Markus Armbruster
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 16/22] libqtest: Add qmp_cmd() helper Eric Blake
2017-08-09 15:40   ` Markus Armbruster
2017-08-09 16:39     ` Eric Blake
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 17/22] libqtest: Add qmp_args() helper Eric Blake
2017-08-09 15:57   ` Markus Armbruster
2017-08-09 21:57     ` Eric Blake
2017-08-10  8:17       ` Markus Armbruster
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 18/22] tests/libqos/usb: Clean up string interpolation into QMP input Eric Blake
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 19/22] libqtest: Add qmp_args_dict() helper Eric Blake
2017-08-09 15:59   ` Markus Armbruster
2017-08-09 16:41     ` Eric Blake
2017-08-10  8:19       ` Markus Armbruster
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 20/22] tests/libqos/pci: Clean up string interpolation into QMP input Eric Blake
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 21/22] libqtest: Drop now-unused qmp() Eric Blake
2017-08-09 16:01   ` Markus Armbruster
2017-08-09 16:43     ` Eric Blake
2017-08-04  1:25 ` [Qemu-devel] [PATCH v4 22/22] libqtest: Rename qmp_cmd() to qmp() Eric Blake
2017-08-04  1:54 ` [Qemu-devel] [PATCH v4 00/22] Clean up around qmp() and hmp() no-reply
2017-08-04 11:50   ` Eric Blake
2017-08-04 12:10     ` Fam Zheng
2017-08-07  6:43       ` Fam Zheng
2017-08-07  7:33         ` Fam Zheng
2017-08-07 14:08           ` Philippe Mathieu-Daudé
2017-08-07  8:09   ` Fam Zheng
2017-08-04  2:02 ` no-reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87lgkurtiw.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.