[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: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v12 17/36] qapi: Fix c_name() munging |
Date: |
Wed, 18 Nov 2015 18:59:37 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> 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).
The truly sane way to compile C with a C++ compiler is of course "not":
compile it with C and link it.
The exception is headers. Stick to declaring extern symbols and the
types they need.
>>> 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.
I reported it, and was told it's been fixed.
- [Qemu-devel] [PATCH] fixup! 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