[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v12 17/36] qapi: Fix c_name() munging
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v12 17/36] qapi: Fix c_name() munging |
Date: |
Wed, 18 Nov 2015 09:20:44 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 11/18/2015 03:18 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> *** WARNING: THE ATTACHED DOCUMENT(S) CONTAIN MACROS ***
>> *** MACROS MAY CONTAIN MALICIOUS CODE ***
>> *** Open only if you can verify and trust the sender ***
>> *** Please contact address@hidden if you have questions or concerns **
>
> Another one.
>
>> The method c_name() is supposed to do two different actions: munge
>> '-' into '_', and add a 'q_' prefix to ticklish names. But it did
>> these steps out of order, making it possible to submit input that
>> is not ticklish until after munging, where the output then lacked
>> the desired prefix.
>>
>> The failure is exposed easily if you have a compiler that recognizes
>> C11 keywords, and try to name a member '_Thread-local', as it would
>> result in trying to compile the declaration 'uint64_t _Thread_local;'
>> which is not valid. However, this name violates our conventions
>> (ultimately, want to enforce that no qapi names start with single
>> underscore), so the test is slightly weaker by instead testing
>> 'wchar-t'; the declaration 'uint64_t wchar_t;' is valid in C (where
>> wchar_t is only a typedef) but fails with a C++ compiler (where it
>> is a keyword).
>
> Do we support compiling it with a C++ compiler? To sidestep the
> question, I'd say "but would fail".
I know we require a C++ compiler for qga on Windows, and qga uses qapi,
so I think the problem is real; but as I do not have a working setup for
compiling qga for windows, I can only speculate. Changing s/fails/but
would fail/ is a nice hedge.
>
> In my private opinion, the one sane way to compile C code with a C++
> compiler is wrapping it in extern "C" { ... }.
True - but I don't think that changes the set of C++ keywords inside the
extern block, does it? (The thought of context-sensitive keywords makes
me cringe for how one would write a sane parser for that language).
>
>> Fix things by reversing the order of actions within c_name().
>>
>> Signed-off-by: Eric Blake <address@hidden>
>
> Again, just commit message polish, can do on merge.
>
Don't know why you got it on some messages and not others; I also got a
round of those pollutions on other threads. It looks like the
responsible party has cleaned up their false positives in the meantime,
so hopefully we aren't hit by more of the annoyance.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v12 21/36] qapi: Tighten the regex on valid names, (continued)
- [Qemu-devel] [PATCH v12 06/36] qapi: Clean up after previous commit, Eric Blake, 2015/11/18
- [Qemu-devel] [PATCH v12 07/36] qapi: Fix up commit 7618b91's clash sanity checking change, Eric Blake, 2015/11/18
- [Qemu-devel] [PATCH v12 12/36] qapi: Factor out QAPISchemaObjectType.check_clash(), Eric Blake, 2015/11/18
- [Qemu-devel] [PATCH v12 18/36] qapi: Remove dead visitor code, Eric Blake, 2015/11/18
- [Qemu-devel] [PATCH v12 16/36] qapi: Detect collisions in C member names, Eric Blake, 2015/11/18
- [Qemu-devel] [PATCH v12 17/36] qapi: Fix c_name() munging, Eric Blake, 2015/11/18
- [Qemu-devel] [PATCH v12 26/36] qapi: Change munging of CamelCase enum values, Eric Blake, 2015/11/18
- [Qemu-devel] [PATCH v12 14/36] qapi: Remove outdated tests related to QMP/branch collisions, Eric Blake, 2015/11/18
- [Qemu-devel] [PATCH v12 23/36] qapi: Remove dead tests for max collision, Eric Blake, 2015/11/18
- [Qemu-devel] [PATCH v12 32/36] qapi: Inline _make_implicit_tag(), Eric Blake, 2015/11/18
- [Qemu-devel] [PATCH v12 30/36] qapi: Convert QType into qapi builtin enum type, Eric Blake, 2015/11/18