On Fri, May 24, 2013 at 05:25:21PM +0200, Markus Armbruster wrote: > Tomoki Sekiyama <tomoki.sekiy...@hds.com> writes: > > > On 5/24/13 4:52 , "Stefan Hajnoczi" <stefa...@gmail.com> wrote: > > > >>On Thu, May 23, 2013 at 06:34:43PM +0000, Tomoki Sekiyama wrote: > >>> On 5/23/13 8:12 , "Stefan Hajnoczi" <stefa...@gmail.com> 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 <tomoki.sekiy...@hds.com> > >>> >> --- > >>> >> 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.
I see what you guys are saying now. You want to protect only qemu-ga. I was proposing protecting the entire codebase so we never get into a situation where changing a header breaks qemu-ga. It's not that hard to use "klass" instead of "class", but if it's unpopular then we'll just have to live with VSS breakage. Stefan