Re: [Qemu-devel] [RFC PATCH v3 02/11] Fix errors and warnings while compiling with c++ compilier

[Markus Armbruster]

Tomoki Sekiyama <address@hidden> writes:

> On 5/24/13 4:52 , "Stefan Hajnoczi" <address@hidden> wrote:
>
>>On Thu, May 23, 2013 at 06:34:43PM +0000, Tomoki Sekiyama wrote:
>>> On 5/23/13 8:12 , "Stefan Hajnoczi" <address@hidden> wrote:
>>> 
>>> >On Tue, May 21, 2013 at 11:33:41AM -0400, Tomoki Sekiyama wrote:
>>> >> Add C++ keywords to avoid errors in compiling with c++ compiler.
>>> >> This also renames class member of PciDeviceInfo to q_class.
>>> >> 
>>> >> Signed-off-by: Tomoki Sekiyama <address@hidden>
>>> >> ---
>>> >>  hmp.c           |    2 +-
>>> >>  hw/pci/pci.c    |    2 +-
>>> >>  scripts/qapi.py |    9 ++++++++-
>>> >>  3 files changed, 10 insertions(+), 3 deletions(-)
>>> >
>>> >Please also extend scripts/checkpatch.pl.  Otherwise it is very likely
>>> >that C++ keywords will be introduced again in the future.  Most people
>>> >will not build the VSS code and therefore checkpatch.pl needs to ensure
>>> >that patches with C++ keywords will not be accepted.
>>> >
>>> >Stefan
>>> 
>>> I think it would be difficult to identify problematic C++ keywords usage
>>> from patches because headers can legally contain C++ keywords and
>>> checkpatch.pl doesn't know where it should be used.
>>> Do you have any good ideas?
>>
>>We can ignore false warnings for 0.1% of patches (the ones that touch
>>VSS C++ code).  But for the remaining 99.9% of patches it's worth
>>guarding against VSS bitrot.
>>
>>Remember not many people will compile it and therefore they won't notice
>>when they break it.  I really think it's worth putting some effort in
>>now so VSS doesn't periodically break.
>>
>>checkpatch.pl is a hacky sort of C parser.  It already checks for a
>>bunch of similar things and it knows about comments, macros, and
>>strings.  It does not perform #include expansion, so there is no risk of
>>including system headers that have C++ code.
>>
>>Stefan
>
> Thanks for your comment.
>
> I'm still wondering because it actually causes a lot false positives
> (not just 0.1%...) even for the patches not touching VSS.
>
> For example, keyword 'class' is used in qdev-monitor.c, qom/object.c,
> and a lot of files in hw/*/*.c and include/{hw,qom}/*.h, but
> they have nothing to do with qemu-ga. Qemu-ga is just a part of whole
> qemu source code, so I don't want to warn around the other parts.

And I appreciate that.  Purging some other language's keywords feels
like pointless churn to me.