From mboxrd@z Thu Jan 1 00:00:00 1970 Received: by 10.28.91.67 with SMTP id p64csp5258630wmb; Wed, 21 Mar 2018 09:22:35 -0700 (PDT) X-Google-Smtp-Source: AIpwx4/a0e61FXX3nF0/DbDU9Ig5OePY88ZxO3BXvEABHzxWK7B3zd512cXm3Dd9YX5V9OYEOKox X-Received: by 10.55.19.154 with SMTP id 26mr401641qkt.172.1521649355408; Wed, 21 Mar 2018 09:22:35 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1521649355; cv=none; d=google.com; s=arc-20160816; b=SG/jUkCoEE9KybH6mG/kvEQNZVdyI7qMymdyH2DKipRiHjP+0Wn4TrPwfCnWr9EE2w X9bE9KZ5a4NWr1PwRoMH4qBsdw2LMuVbU4tiZyxklzawR8nTzIhVigyIjJjawLc7yGBr WVE4hkVolYLfOm8j730U5pNYNMVeMvgNJsXBA3r/kv0iZGri2W27twLS5KHy4xHTsnnY WEQ7QDNysUz5oEkNAkp57/LwR5glEZQ28eq39vwcx+E1aPcC5lpZlv8bqG0V+I3sE5FU J7B5YPJZG7qPmilYwSJFsSbAv581o49hZv6k6Urg251JFd3L+do4wgXuE3dlNogHrK6h awhQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:subject:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:to:from:date :arc-authentication-results; bh=trBvevQbQFnVf3i+yCv12OIkO/vyXhDH5a/4HtnXeQc=; b=bqxBt7j8cm7Kvz16jXzACcbajXEBkwHQRv8D+H7oQy30VLUCx0D58yu4cOvtO4Hfcr dH2ZnjOmi9L9Xx6LQI7sCcLLV2EcpUU+cz1htSV+UOzL0OHb5ICduzIQkDs6BBVt3HMt mv/rZ/nA9wpsZ8JQkKba1TouF3k3r02HvMV0O8CQ3xZGtKUHvGpJ87vBynEFRAxt32zI DupxEaR4Zxg4d7g8cvFtux0pbrtFRJubZdss6shpTFkxiO+gao+QwhVwac+FQWKH4wLG slqfykiFLrWDyAob2kPyTzY9KtgJ+NNKeWvjasH8Q7seOSg6biq/24gRNq04vI0+1caB h8aQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom=qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id u40si3566765qth.72.2018.03.21.09.22.35 for (version=TLS1 cipher=AES128-SHA bits=128/128); Wed, 21 Mar 2018 09:22:35 -0700 (PDT) Received-SPF: pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Authentication-Results: mx.google.com; spf=pass (google.com: domain of qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org designates 2001:4830:134:3::11 as permitted sender) smtp.mailfrom=qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from localhost ([::1]:56155 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eygVW-0000UO-RD for alex.bennee@linaro.org; Wed, 21 Mar 2018 12:22:34 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39144) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eygVJ-0000TA-Ek for qemu-arm@nongnu.org; Wed, 21 Mar 2018 12:22:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eygVG-0007h4-6l for qemu-arm@nongnu.org; Wed, 21 Mar 2018 12:22:21 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:55790 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eygVG-0007gv-0x; Wed, 21 Mar 2018 12:22:18 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ED98D6166F; Wed, 21 Mar 2018 16:22:16 +0000 (UTC) Received: from localhost.localdomain (ovpn-117-20.ams2.redhat.com [10.36.117.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A194D215CDA7; Wed, 21 Mar 2018 16:22:04 +0000 (UTC) Date: Wed, 21 Mar 2018 17:22:03 +0100 From: Kevin Wolf To: "Michael S. Tsirkin" Message-ID: <20180321162203.GE3898@localhost.localdomain> References: <1521642402-197739-1-git-send-email-mst@redhat.com> <20180321153439.GC3898@localhost.localdomain> <20180321175452-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180321175452-mutt-send-email-mst@kernel.org> User-Agent: Mutt/1.9.1 (2017-09-22) X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Wed, 21 Mar 2018 16:22:17 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Wed, 21 Mar 2018 16:22:17 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'kwolf@redhat.com' RCPT:'' X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 66.187.233.73 Subject: Re: [Qemu-arm] [PATCH v2] qemu: replace "" with <> in headers X-BeenThere: qemu-arm@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Maydell , Dmitry Fleytman , Pavel Dovgalyuk , Li Zhijian , David Hildenbrand , Jeff Cody , Stefan Hajnoczi , qemu-devel@nongnu.org, BALATON Zoltan , Keith Busch , Max Filippov , Hannes Reinecke , Gerd Hoffmann , Fam Zheng , Max Reitz , Eric Blake , Josh Durgin , Stefano Stabellini , Alberto Garcia , zhanghailiang , Ben Warren , Stefan Berger , Yongbok Kim , Michael Roth , "Richard W.M. Jones" , Christian Borntraeger , =?iso-8859-1?Q?Herv=E9?= Poussineau , =?iso-8859-1?Q?Marc-Andr=E9?= Lureau , Shannon Zhao , Marcel Apfelbaum , Liu Yuan , Richard Henderson , Andrzej Zaborowski , Jason Wang , Artyom Tarasenko , Thomas Huth , Alistair Francis , Jiri Pirko , Eduardo Habkost , Corey Minyard , Amit Shah , Stefan Weil , Xie Changlong , Riku Voipio , Peter Lieven , "Dr. David Alan Gilbert" , Yuval Shaia , Greg Kurz , Anthony Perard , Alex Williamson , qemu-arm@nongnu.org, Peter Chubb , Ronnie Sahlberg , Zhang Chen , xen-devel@lists.xenproject.org, John Snow , David Gibson , kvm@vger.kernel.org, Daniel =?iso-8859-1?Q?P=2E_Berrang=E9?= , qemu-block@nongnu.org, Hitoshi Mitake , Markus Armbruster , qemu-s390x@nongnu.org, Marcelo Tosatti , Laurent Vivier , Juan Quintela , Subbaraya Sundeep , Michael Walle , Igor Mammedov , qemu-ppc@nongnu.org, Wen Congyang , Cornelia Huck , Paolo Bonzini , Andreas =?iso-8859-1?Q?F=E4rber?= , Philippe =?iso-8859-1?Q?Mathieu-Daud=E9?= Errors-To: qemu-arm-bounces+alex.bennee=linaro.org@nongnu.org Sender: "Qemu-arm" X-TUID: rnunkNZPoNc4 Am 21.03.2018 um 16:58 hat Michael S. Tsirkin geschrieben: > On Wed, Mar 21, 2018 at 04:34:39PM +0100, Kevin Wolf wrote: > > Am 21.03.2018 um 15:46 hat Michael S. Tsirkin geschrieben: > > > Our current scheme is to use > > > #include "" > > > for internal headers, and > > > #include <> > > > for external ones. > > > > > > Unfortunately this is not based on compiler support: from C point of > > > view, the "" form merely looks up headers in the current directory > > > and then falls back on <> directories. > > > > > > Thus, for example, a system header trace.h - should it be present - will > > > conflict with our local trace.h > > > > You're right that there is a conflict, even though only in one > > direction: "trace.h" is unambiguously the local trace.h in our source > > tree, but refers to the same local header rather than the > > system header as you would expect. > > > > An easy way to resolve this conflict would be using -iquote rather than > > -I for directories in the source tree, so that unambiguously > > refers to the system header and "trace.h" unambiguously refers to the > > QEMU header. > > I posted patches to that effect for 2.12. Ah, I missed those. That's good (and imho enough). > It's all still very much a non-standard convention and so less robust > than prefixing file name with a project-specifix prefix. I've always had the impression that it's by far the most common convention, to the point that I'd blindly assume it when joining a new project. > > > As another example of problems, a header by the same name in the source > > > directory will always be picked up first - before any headers in > > > the include directory. > > > > > > Let's change the scheme: make sure all headers that are not > > > in the source directory are included through a path > > > starting with qemu/ , thus: > > > > > > #include <> > > > > > > headers in the same directory as source are included with > > > > > > #include "" > > > > > > as per standard. > > > > > > This (untested) patch is just to start the discussion and does not > > > change all of the codebase. If there's agreement, this will be > > > run on all code to converting code to this scheme. > > > > Renaming files is always painful. If that's the fix, the cure might be > > worse than the disease. As far as I know, the conflict is only > > theoretical, so in that case I'd say: If it ain't broke, don't fix it. > > > > Kevin > > It's broke I think, it's very hard for new people to contribute to QEMU. > Look e.g. at rdma which all has messed up includes - and that's from an > experienced conributor who just isn't an experienced maintainer. I don't think the problem is that the convention is hard to apply (it's definitely not). It's knowing about the convention. This problem isn't going away by switching to a different, less common convention. We're only going to see more offenders then. > Amount of time spent on teaching new people trivia about our > conventions just isn't funny. They should be self-documenting > and violations should cause the build to fail. Yes, but your proposal doesn't achieve this. You can still use "qemu/foo.h" instead of and it will build successfully. That's something we can't change, as far as I know, because the include path for "foo.h" is always a superset of . If anything, this means that we should prefer "foo.h" for local headers (i.e. the way it currently is) because we can let the compiler enforce it: for "foo.h" can become a build error, and does so with your -iquote patch, but the other way round doesn't work. Then it's only system headers that you can possibly get wrong, but for those everyone should be used to using anyway. Kevin From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Wolf Subject: Re: [PATCH v2] qemu: replace "" with <> in headers Date: Wed, 21 Mar 2018 17:22:03 +0100 Message-ID: <20180321162203.GE3898@localhost.localdomain> References: <1521642402-197739-1-git-send-email-mst@redhat.com> <20180321153439.GC3898@localhost.localdomain> <20180321175452-mutt-send-email-mst@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from all-amaz-eas1.inumbo.com ([34.197.232.57]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1eygVG-0000sA-Vm for xen-devel@lists.xenproject.org; Wed, 21 Mar 2018 16:22:19 +0000 Content-Disposition: inline In-Reply-To: <20180321175452-mutt-send-email-mst@kernel.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" To: "Michael S. Tsirkin" Cc: Peter Maydell , Dmitry Fleytman , Pavel Dovgalyuk , Li Zhijian , David Hildenbrand , Jeff Cody , Stefan Hajnoczi , qemu-devel@nongnu.org, BALATON Zoltan , Keith Busch , Max Filippov , Hannes Reinecke , Gerd Hoffmann , Fam Zheng , Max Reitz , Eric Blake , Josh Durgin , Stefano Stabellini , Alberto Garcia , zhanghailiang , Ben Warren , Stefan Berger , Yongbok Kim , Michael Roth List-Id: xen-devel@lists.xenproject.org QW0gMjEuMDMuMjAxOCB1bSAxNjo1OCBoYXQgTWljaGFlbCBTLiBUc2lya2luIGdlc2NocmllYmVu Ogo+IE9uIFdlZCwgTWFyIDIxLCAyMDE4IGF0IDA0OjM0OjM5UE0gKzAxMDAsIEtldmluIFdvbGYg d3JvdGU6Cj4gPiBBbSAyMS4wMy4yMDE4IHVtIDE1OjQ2IGhhdCBNaWNoYWVsIFMuIFRzaXJraW4g Z2VzY2hyaWViZW46Cj4gPiA+IE91ciBjdXJyZW50IHNjaGVtZSBpcyB0byB1c2UKPiA+ID4gICNp bmNsdWRlICIiCj4gPiA+IGZvciBpbnRlcm5hbCBoZWFkZXJzLCBhbmQKPiA+ID4gICNpbmNsdWRl IDw+Cj4gPiA+IGZvciBleHRlcm5hbCBvbmVzLgo+ID4gPiAKPiA+ID4gVW5mb3J0dW5hdGVseSB0 aGlzIGlzIG5vdCBiYXNlZCBvbiBjb21waWxlciBzdXBwb3J0OiBmcm9tIEMgcG9pbnQgb2YKPiA+ ID4gdmlldywgdGhlICIiIGZvcm0gbWVyZWx5IGxvb2tzIHVwIGhlYWRlcnMgaW4gdGhlIGN1cnJl bnQgZGlyZWN0b3J5Cj4gPiA+IGFuZCB0aGVuIGZhbGxzIGJhY2sgb24gPD4gZGlyZWN0b3JpZXMu Cj4gPiA+IAo+ID4gPiBUaHVzLCBmb3IgZXhhbXBsZSwgYSBzeXN0ZW0gaGVhZGVyIHRyYWNlLmgg LSBzaG91bGQgaXQgYmUgcHJlc2VudCAtIHdpbGwKPiA+ID4gY29uZmxpY3Qgd2l0aCBvdXIgbG9j YWwgdHJhY2UuaAo+ID4gCj4gPiBZb3UncmUgcmlnaHQgdGhhdCB0aGVyZSBpcyBhIGNvbmZsaWN0 LCBldmVuIHRob3VnaCBvbmx5IGluIG9uZQo+ID4gZGlyZWN0aW9uOiAidHJhY2UuaCIgaXMgdW5h bWJpZ3VvdXNseSB0aGUgbG9jYWwgdHJhY2UuaCBpbiBvdXIgc291cmNlCj4gPiB0cmVlLCBidXQg PHRyYWNlLmg+IHJlZmVycyB0byB0aGUgc2FtZSBsb2NhbCBoZWFkZXIgcmF0aGVyIHRoYW4gdGhl Cj4gPiBzeXN0ZW0gaGVhZGVyIGFzIHlvdSB3b3VsZCBleHBlY3QuCj4gPiAKPiA+IEFuIGVhc3kg d2F5IHRvIHJlc29sdmUgdGhpcyBjb25mbGljdCB3b3VsZCBiZSB1c2luZyAtaXF1b3RlIHJhdGhl ciB0aGFuCj4gPiAtSSBmb3IgZGlyZWN0b3JpZXMgaW4gdGhlIHNvdXJjZSB0cmVlLCBzbyB0aGF0 IDx0cmFjZS5oPiB1bmFtYmlndW91c2x5Cj4gPiByZWZlcnMgdG8gdGhlIHN5c3RlbSBoZWFkZXIg YW5kICJ0cmFjZS5oIiB1bmFtYmlndW91c2x5IHJlZmVycyB0byB0aGUKPiA+IFFFTVUgaGVhZGVy Lgo+IAo+IEkgcG9zdGVkIHBhdGNoZXMgdG8gdGhhdCBlZmZlY3QgZm9yIDIuMTIuCgpBaCwgSSBt aXNzZWQgdGhvc2UuIFRoYXQncyBnb29kIChhbmQgaW1obyBlbm91Z2gpLgoKPiBJdCdzIGFsbCBz dGlsbCB2ZXJ5IG11Y2ggYSBub24tc3RhbmRhcmQgY29udmVudGlvbiBhbmQgc28gbGVzcyByb2J1 c3QKPiB0aGFuIHByZWZpeGluZyBmaWxlIG5hbWUgd2l0aCBhIHByb2plY3Qtc3BlY2lmaXggcHJl Zml4LgoKSSd2ZSBhbHdheXMgaGFkIHRoZSBpbXByZXNzaW9uIHRoYXQgaXQncyBieSBmYXIgdGhl IG1vc3QgY29tbW9uCmNvbnZlbnRpb24sIHRvIHRoZSBwb2ludCB0aGF0IEknZCBibGluZGx5IGFz c3VtZSBpdCB3aGVuIGpvaW5pbmcgYSBuZXcKcHJvamVjdC4KCj4gPiA+IEFzIGFub3RoZXIgZXhh bXBsZSBvZiBwcm9ibGVtcywgYSBoZWFkZXIgYnkgdGhlIHNhbWUgbmFtZSBpbiB0aGUgc291cmNl Cj4gPiA+IGRpcmVjdG9yeSB3aWxsIGFsd2F5cyBiZSBwaWNrZWQgdXAgZmlyc3QgLSBiZWZvcmUg YW55IGhlYWRlcnMgaW4KPiA+ID4gdGhlIGluY2x1ZGUgZGlyZWN0b3J5Lgo+ID4gPiAKPiA+ID4g TGV0J3MgY2hhbmdlIHRoZSBzY2hlbWU6IG1ha2Ugc3VyZSBhbGwgaGVhZGVycyB0aGF0IGFyZSBu b3QKPiA+ID4gaW4gdGhlIHNvdXJjZSBkaXJlY3RvcnkgYXJlIGluY2x1ZGVkIHRocm91Z2ggYSBw YXRoCj4gPiA+IHN0YXJ0aW5nIHdpdGggcWVtdS8gLCB0aHVzOgo+ID4gPiAKPiA+ID4gICNpbmNs dWRlIDw+Cj4gPiA+IAo+ID4gPiBoZWFkZXJzIGluIHRoZSBzYW1lIGRpcmVjdG9yeSBhcyBzb3Vy Y2UgYXJlIGluY2x1ZGVkIHdpdGgKPiA+ID4gCj4gPiA+ICAjaW5jbHVkZSAiIgo+ID4gPiAKPiA+ ID4gYXMgcGVyIHN0YW5kYXJkLgo+ID4gPiAKPiA+ID4gVGhpcyAodW50ZXN0ZWQpIHBhdGNoIGlz IGp1c3QgdG8gc3RhcnQgdGhlIGRpc2N1c3Npb24gYW5kIGRvZXMgbm90Cj4gPiA+IGNoYW5nZSBh bGwgb2YgdGhlIGNvZGViYXNlLiBJZiB0aGVyZSdzIGFncmVlbWVudCwgdGhpcyB3aWxsIGJlCj4g PiA+IHJ1biBvbiBhbGwgY29kZSB0byBjb252ZXJ0aW5nIGNvZGUgdG8gdGhpcyBzY2hlbWUuCj4g PiAKPiA+IFJlbmFtaW5nIGZpbGVzIGlzIGFsd2F5cyBwYWluZnVsLiBJZiB0aGF0J3MgdGhlIGZp eCwgdGhlIGN1cmUgbWlnaHQgYmUKPiA+IHdvcnNlIHRoYW4gdGhlIGRpc2Vhc2UuIEFzIGZhciBh cyBJIGtub3csIHRoZSBjb25mbGljdCBpcyBvbmx5Cj4gPiB0aGVvcmV0aWNhbCwgc28gaW4gdGhh dCBjYXNlIEknZCBzYXk6IElmIGl0IGFpbid0IGJyb2tlLCBkb24ndCBmaXggaXQuCj4gPiAKPiA+ IEtldmluCj4gCj4gSXQncyBicm9rZSBJIHRoaW5rLCBpdCdzIHZlcnkgaGFyZCBmb3IgbmV3IHBl b3BsZSB0byBjb250cmlidXRlIHRvIFFFTVUuCj4gTG9vayBlLmcuIGF0IHJkbWEgd2hpY2ggYWxs IGhhcyBtZXNzZWQgdXAgaW5jbHVkZXMgLSBhbmQgdGhhdCdzIGZyb20gYW4KPiBleHBlcmllbmNl ZCBjb25yaWJ1dG9yIHdobyBqdXN0IGlzbid0IGFuIGV4cGVyaWVuY2VkIG1haW50YWluZXIuCgpJ IGRvbid0IHRoaW5rIHRoZSBwcm9ibGVtIGlzIHRoYXQgdGhlIGNvbnZlbnRpb24gaXMgaGFyZCB0 byBhcHBseSAoaXQncwpkZWZpbml0ZWx5IG5vdCkuIEl0J3Mga25vd2luZyBhYm91dCB0aGUgY29u dmVudGlvbi4gVGhpcyBwcm9ibGVtIGlzbid0CmdvaW5nIGF3YXkgYnkgc3dpdGNoaW5nIHRvIGEg ZGlmZmVyZW50LCBsZXNzIGNvbW1vbiBjb252ZW50aW9uLiBXZSdyZQpvbmx5IGdvaW5nIHRvIHNl ZSBtb3JlIG9mZmVuZGVycyB0aGVuLgoKPiBBbW91bnQgb2YgdGltZSBzcGVudCBvbiB0ZWFjaGlu ZyBuZXcgcGVvcGxlIHRyaXZpYSBhYm91dCBvdXIKPiBjb252ZW50aW9ucyBqdXN0IGlzbid0IGZ1 bm55LiBUaGV5IHNob3VsZCBiZSBzZWxmLWRvY3VtZW50aW5nCj4gYW5kIHZpb2xhdGlvbnMgc2hv dWxkIGNhdXNlIHRoZSBidWlsZCB0byBmYWlsLgoKWWVzLCBidXQgeW91ciBwcm9wb3NhbCBkb2Vz bid0IGFjaGlldmUgdGhpcy4gWW91IGNhbiBzdGlsbCB1c2UKInFlbXUvZm9vLmgiIGluc3RlYWQg b2YgPHFlbXUvZm9vLmg+IGFuZCBpdCB3aWxsIGJ1aWxkIHN1Y2Nlc3NmdWxseS4KVGhhdCdzIHNv bWV0aGluZyB3ZSBjYW4ndCBjaGFuZ2UsIGFzIGZhciBhcyBJIGtub3csIGJlY2F1c2UgdGhlIGlu Y2x1ZGUKcGF0aCBmb3IgImZvby5oIiBpcyBhbHdheXMgYSBzdXBlcnNldCBvZiA8Zm9vLmg+LgoK SWYgYW55dGhpbmcsIHRoaXMgbWVhbnMgdGhhdCB3ZSBzaG91bGQgcHJlZmVyICJmb28uaCIgZm9y IGxvY2FsIGhlYWRlcnMKKGkuZS4gdGhlIHdheSBpdCBjdXJyZW50bHkgaXMpIGJlY2F1c2Ugd2Ug Y2FuIGxldCB0aGUgY29tcGlsZXIgZW5mb3JjZQppdDogPGZvby5oPiBmb3IgImZvby5oIiBjYW4g YmVjb21lIGEgYnVpbGQgZXJyb3IsIGFuZCBkb2VzIHNvIHdpdGggeW91cgotaXF1b3RlIHBhdGNo LCBidXQgdGhlIG90aGVyIHdheSByb3VuZCBkb2Vzbid0IHdvcmsuCgpUaGVuIGl0J3Mgb25seSBz eXN0ZW0gaGVhZGVycyB0aGF0IHlvdSBjYW4gcG9zc2libHkgZ2V0IHdyb25nLCBidXQgZm9yCnRo b3NlIGV2ZXJ5b25lIHNob3VsZCBiZSB1c2VkIHRvIHVzaW5nIDxmb28uaD4gYW55d2F5LgoKS2V2 aW4KCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fClhlbi1k ZXZlbCBtYWlsaW5nIGxpc3QKWGVuLWRldmVsQGxpc3RzLnhlbnByb2plY3Qub3JnCmh0dHBzOi8v bGlzdHMueGVucHJvamVjdC5vcmcvbWFpbG1hbi9saXN0aW5mby94ZW4tZGV2ZWw= From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Wolf Subject: Re: [PATCH v2] qemu: replace "" with <> in headers Date: Wed, 21 Mar 2018 17:22:03 +0100 Message-ID: <20180321162203.GE3898@localhost.localdomain> References: <1521642402-197739-1-git-send-email-mst@redhat.com> <20180321153439.GC3898@localhost.localdomain> <20180321175452-mutt-send-email-mst@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Cc: Peter Maydell , Dmitry Fleytman , Pavel Dovgalyuk , Li Zhijian , David Hildenbrand , Jeff Cody , Stefan Hajnoczi , qemu-devel@nongnu.org, BALATON Zoltan , Keith Busch , Max Filippov , Hannes Reinecke , Gerd Hoffmann , Fam Zheng , Max Reitz , Eric Blake , Josh Durgin , Stefano Stabellini , Alberto Garcia , zhanghailiang , Ben Warren , Stefan Berger , Yongbok Kim , Michael Roth Return-path: Content-Disposition: inline In-Reply-To: <20180321175452-mutt-send-email-mst@kernel.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" List-Id: kvm.vger.kernel.org QW0gMjEuMDMuMjAxOCB1bSAxNjo1OCBoYXQgTWljaGFlbCBTLiBUc2lya2luIGdlc2NocmllYmVu Ogo+IE9uIFdlZCwgTWFyIDIxLCAyMDE4IGF0IDA0OjM0OjM5UE0gKzAxMDAsIEtldmluIFdvbGYg d3JvdGU6Cj4gPiBBbSAyMS4wMy4yMDE4IHVtIDE1OjQ2IGhhdCBNaWNoYWVsIFMuIFRzaXJraW4g Z2VzY2hyaWViZW46Cj4gPiA+IE91ciBjdXJyZW50IHNjaGVtZSBpcyB0byB1c2UKPiA+ID4gICNp bmNsdWRlICIiCj4gPiA+IGZvciBpbnRlcm5hbCBoZWFkZXJzLCBhbmQKPiA+ID4gICNpbmNsdWRl IDw+Cj4gPiA+IGZvciBleHRlcm5hbCBvbmVzLgo+ID4gPiAKPiA+ID4gVW5mb3J0dW5hdGVseSB0 aGlzIGlzIG5vdCBiYXNlZCBvbiBjb21waWxlciBzdXBwb3J0OiBmcm9tIEMgcG9pbnQgb2YKPiA+ ID4gdmlldywgdGhlICIiIGZvcm0gbWVyZWx5IGxvb2tzIHVwIGhlYWRlcnMgaW4gdGhlIGN1cnJl bnQgZGlyZWN0b3J5Cj4gPiA+IGFuZCB0aGVuIGZhbGxzIGJhY2sgb24gPD4gZGlyZWN0b3JpZXMu Cj4gPiA+IAo+ID4gPiBUaHVzLCBmb3IgZXhhbXBsZSwgYSBzeXN0ZW0gaGVhZGVyIHRyYWNlLmgg LSBzaG91bGQgaXQgYmUgcHJlc2VudCAtIHdpbGwKPiA+ID4gY29uZmxpY3Qgd2l0aCBvdXIgbG9j YWwgdHJhY2UuaAo+ID4gCj4gPiBZb3UncmUgcmlnaHQgdGhhdCB0aGVyZSBpcyBhIGNvbmZsaWN0 LCBldmVuIHRob3VnaCBvbmx5IGluIG9uZQo+ID4gZGlyZWN0aW9uOiAidHJhY2UuaCIgaXMgdW5h bWJpZ3VvdXNseSB0aGUgbG9jYWwgdHJhY2UuaCBpbiBvdXIgc291cmNlCj4gPiB0cmVlLCBidXQg PHRyYWNlLmg+IHJlZmVycyB0byB0aGUgc2FtZSBsb2NhbCBoZWFkZXIgcmF0aGVyIHRoYW4gdGhl Cj4gPiBzeXN0ZW0gaGVhZGVyIGFzIHlvdSB3b3VsZCBleHBlY3QuCj4gPiAKPiA+IEFuIGVhc3kg d2F5IHRvIHJlc29sdmUgdGhpcyBjb25mbGljdCB3b3VsZCBiZSB1c2luZyAtaXF1b3RlIHJhdGhl ciB0aGFuCj4gPiAtSSBmb3IgZGlyZWN0b3JpZXMgaW4gdGhlIHNvdXJjZSB0cmVlLCBzbyB0aGF0 IDx0cmFjZS5oPiB1bmFtYmlndW91c2x5Cj4gPiByZWZlcnMgdG8gdGhlIHN5c3RlbSBoZWFkZXIg YW5kICJ0cmFjZS5oIiB1bmFtYmlndW91c2x5IHJlZmVycyB0byB0aGUKPiA+IFFFTVUgaGVhZGVy Lgo+IAo+IEkgcG9zdGVkIHBhdGNoZXMgdG8gdGhhdCBlZmZlY3QgZm9yIDIuMTIuCgpBaCwgSSBt aXNzZWQgdGhvc2UuIFRoYXQncyBnb29kIChhbmQgaW1obyBlbm91Z2gpLgoKPiBJdCdzIGFsbCBz dGlsbCB2ZXJ5IG11Y2ggYSBub24tc3RhbmRhcmQgY29udmVudGlvbiBhbmQgc28gbGVzcyByb2J1 c3QKPiB0aGFuIHByZWZpeGluZyBmaWxlIG5hbWUgd2l0aCBhIHByb2plY3Qtc3BlY2lmaXggcHJl Zml4LgoKSSd2ZSBhbHdheXMgaGFkIHRoZSBpbXByZXNzaW9uIHRoYXQgaXQncyBieSBmYXIgdGhl IG1vc3QgY29tbW9uCmNvbnZlbnRpb24sIHRvIHRoZSBwb2ludCB0aGF0IEknZCBibGluZGx5IGFz c3VtZSBpdCB3aGVuIGpvaW5pbmcgYSBuZXcKcHJvamVjdC4KCj4gPiA+IEFzIGFub3RoZXIgZXhh bXBsZSBvZiBwcm9ibGVtcywgYSBoZWFkZXIgYnkgdGhlIHNhbWUgbmFtZSBpbiB0aGUgc291cmNl Cj4gPiA+IGRpcmVjdG9yeSB3aWxsIGFsd2F5cyBiZSBwaWNrZWQgdXAgZmlyc3QgLSBiZWZvcmUg YW55IGhlYWRlcnMgaW4KPiA+ID4gdGhlIGluY2x1ZGUgZGlyZWN0b3J5Lgo+ID4gPiAKPiA+ID4g TGV0J3MgY2hhbmdlIHRoZSBzY2hlbWU6IG1ha2Ugc3VyZSBhbGwgaGVhZGVycyB0aGF0IGFyZSBu b3QKPiA+ID4gaW4gdGhlIHNvdXJjZSBkaXJlY3RvcnkgYXJlIGluY2x1ZGVkIHRocm91Z2ggYSBw YXRoCj4gPiA+IHN0YXJ0aW5nIHdpdGggcWVtdS8gLCB0aHVzOgo+ID4gPiAKPiA+ID4gICNpbmNs dWRlIDw+Cj4gPiA+IAo+ID4gPiBoZWFkZXJzIGluIHRoZSBzYW1lIGRpcmVjdG9yeSBhcyBzb3Vy Y2UgYXJlIGluY2x1ZGVkIHdpdGgKPiA+ID4gCj4gPiA+ICAjaW5jbHVkZSAiIgo+ID4gPiAKPiA+ ID4gYXMgcGVyIHN0YW5kYXJkLgo+ID4gPiAKPiA+ID4gVGhpcyAodW50ZXN0ZWQpIHBhdGNoIGlz IGp1c3QgdG8gc3RhcnQgdGhlIGRpc2N1c3Npb24gYW5kIGRvZXMgbm90Cj4gPiA+IGNoYW5nZSBh bGwgb2YgdGhlIGNvZGViYXNlLiBJZiB0aGVyZSdzIGFncmVlbWVudCwgdGhpcyB3aWxsIGJlCj4g PiA+IHJ1biBvbiBhbGwgY29kZSB0byBjb252ZXJ0aW5nIGNvZGUgdG8gdGhpcyBzY2hlbWUuCj4g PiAKPiA+IFJlbmFtaW5nIGZpbGVzIGlzIGFsd2F5cyBwYWluZnVsLiBJZiB0aGF0J3MgdGhlIGZp eCwgdGhlIGN1cmUgbWlnaHQgYmUKPiA+IHdvcnNlIHRoYW4gdGhlIGRpc2Vhc2UuIEFzIGZhciBh cyBJIGtub3csIHRoZSBjb25mbGljdCBpcyBvbmx5Cj4gPiB0aGVvcmV0aWNhbCwgc28gaW4gdGhh dCBjYXNlIEknZCBzYXk6IElmIGl0IGFpbid0IGJyb2tlLCBkb24ndCBmaXggaXQuCj4gPiAKPiA+ IEtldmluCj4gCj4gSXQncyBicm9rZSBJIHRoaW5rLCBpdCdzIHZlcnkgaGFyZCBmb3IgbmV3IHBl b3BsZSB0byBjb250cmlidXRlIHRvIFFFTVUuCj4gTG9vayBlLmcuIGF0IHJkbWEgd2hpY2ggYWxs IGhhcyBtZXNzZWQgdXAgaW5jbHVkZXMgLSBhbmQgdGhhdCdzIGZyb20gYW4KPiBleHBlcmllbmNl ZCBjb25yaWJ1dG9yIHdobyBqdXN0IGlzbid0IGFuIGV4cGVyaWVuY2VkIG1haW50YWluZXIuCgpJ IGRvbid0IHRoaW5rIHRoZSBwcm9ibGVtIGlzIHRoYXQgdGhlIGNvbnZlbnRpb24gaXMgaGFyZCB0 byBhcHBseSAoaXQncwpkZWZpbml0ZWx5IG5vdCkuIEl0J3Mga25vd2luZyBhYm91dCB0aGUgY29u dmVudGlvbi4gVGhpcyBwcm9ibGVtIGlzbid0CmdvaW5nIGF3YXkgYnkgc3dpdGNoaW5nIHRvIGEg ZGlmZmVyZW50LCBsZXNzIGNvbW1vbiBjb252ZW50aW9uLiBXZSdyZQpvbmx5IGdvaW5nIHRvIHNl ZSBtb3JlIG9mZmVuZGVycyB0aGVuLgoKPiBBbW91bnQgb2YgdGltZSBzcGVudCBvbiB0ZWFjaGlu ZyBuZXcgcGVvcGxlIHRyaXZpYSBhYm91dCBvdXIKPiBjb252ZW50aW9ucyBqdXN0IGlzbid0IGZ1 bm55LiBUaGV5IHNob3VsZCBiZSBzZWxmLWRvY3VtZW50aW5nCj4gYW5kIHZpb2xhdGlvbnMgc2hv dWxkIGNhdXNlIHRoZSBidWlsZCB0byBmYWlsLgoKWWVzLCBidXQgeW91ciBwcm9wb3NhbCBkb2Vz bid0IGFjaGlldmUgdGhpcy4gWW91IGNhbiBzdGlsbCB1c2UKInFlbXUvZm9vLmgiIGluc3RlYWQg b2YgPHFlbXUvZm9vLmg+IGFuZCBpdCB3aWxsIGJ1aWxkIHN1Y2Nlc3NmdWxseS4KVGhhdCdzIHNv bWV0aGluZyB3ZSBjYW4ndCBjaGFuZ2UsIGFzIGZhciBhcyBJIGtub3csIGJlY2F1c2UgdGhlIGlu Y2x1ZGUKcGF0aCBmb3IgImZvby5oIiBpcyBhbHdheXMgYSBzdXBlcnNldCBvZiA8Zm9vLmg+LgoK SWYgYW55dGhpbmcsIHRoaXMgbWVhbnMgdGhhdCB3ZSBzaG91bGQgcHJlZmVyICJmb28uaCIgZm9y IGxvY2FsIGhlYWRlcnMKKGkuZS4gdGhlIHdheSBpdCBjdXJyZW50bHkgaXMpIGJlY2F1c2Ugd2Ug Y2FuIGxldCB0aGUgY29tcGlsZXIgZW5mb3JjZQppdDogPGZvby5oPiBmb3IgImZvby5oIiBjYW4g YmVjb21lIGEgYnVpbGQgZXJyb3IsIGFuZCBkb2VzIHNvIHdpdGggeW91cgotaXF1b3RlIHBhdGNo LCBidXQgdGhlIG90aGVyIHdheSByb3VuZCBkb2Vzbid0IHdvcmsuCgpUaGVuIGl0J3Mgb25seSBz eXN0ZW0gaGVhZGVycyB0aGF0IHlvdSBjYW4gcG9zc2libHkgZ2V0IHdyb25nLCBidXQgZm9yCnRo b3NlIGV2ZXJ5b25lIHNob3VsZCBiZSB1c2VkIHRvIHVzaW5nIDxmb28uaD4gYW55d2F5LgoKS2V2 aW4KCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fClhlbi1k ZXZlbCBtYWlsaW5nIGxpc3QKWGVuLWRldmVsQGxpc3RzLnhlbnByb2plY3Qub3JnCmh0dHBzOi8v bGlzdHMueGVucHJvamVjdC5vcmcvbWFpbG1hbi9saXN0aW5mby94ZW4tZGV2ZWw= From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39213) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eygVU-0000Zw-12 for qemu-devel@nongnu.org; Wed, 21 Mar 2018 12:22:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eygVS-0007k7-KS for qemu-devel@nongnu.org; Wed, 21 Mar 2018 12:22:32 -0400 Date: Wed, 21 Mar 2018 17:22:03 +0100 From: Kevin Wolf Message-ID: <20180321162203.GE3898@localhost.localdomain> References: <1521642402-197739-1-git-send-email-mst@redhat.com> <20180321153439.GC3898@localhost.localdomain> <20180321175452-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180321175452-mutt-send-email-mst@kernel.org> Subject: Re: [Qemu-devel] [PATCH v2] qemu: replace "" with <> in headers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org, Daniel =?iso-8859-1?Q?P=2E_Berrang=E9?= , Thomas Huth , Laurent Vivier , Peter Maydell , Dmitry Fleytman , Ronnie Sahlberg , Li Zhijian , David Hildenbrand , Jeff Cody , Zhang Chen , BALATON Zoltan , Keith Busch , Max Filippov , Gerd Hoffmann , Jiri Pirko , Subbaraya Sundeep , Eric Blake , Michael Roth , Marcelo Tosatti , Josh Durgin , Stefano Stabellini , Alberto Garcia , zhanghailiang , Ben Warren , Marcel Apfelbaum , Yongbok Kim , Markus Armbruster , Stefan Berger , Christian Borntraeger , kvm@vger.kernel.org, =?iso-8859-1?Q?Herv=E9?= Poussineau , Shannon Zhao , Anthony Perard , Liu Yuan , David Gibson , Andrzej Zaborowski , Jason Wang , Artyom Tarasenko , Riku Voipio , Fam Zheng , Eduardo Habkost , Corey Minyard , Amit Shah , Pavel Dovgalyuk , Stefan Weil , Xie Changlong , Alistair Francis , Peter Lieven , "Dr. David Alan Gilbert" , Greg Kurz , =?iso-8859-1?Q?Marc-Andr=E9?= Lureau , Alex Williamson , qemu-arm@nongnu.org, Peter Chubb , Yuval Shaia , Stefan Hajnoczi , Paolo Bonzini , xen-devel@lists.xenproject.org, John Snow , Richard Henderson , qemu-block@nongnu.org, Peter Crosthwaite , Hitoshi Mitake , Wen Congyang , qemu-s390x@nongnu.org, Cornelia Huck , "Richard W.M. Jones" , Juan Quintela , Max Reitz , Michael Walle , qemu-ppc@nongnu.org, Andreas =?iso-8859-1?Q?F=E4rber?= , Igor Mammedov , Hannes Reinecke , Philippe =?iso-8859-1?Q?Mathieu-Daud=E9?= Am 21.03.2018 um 16:58 hat Michael S. Tsirkin geschrieben: > On Wed, Mar 21, 2018 at 04:34:39PM +0100, Kevin Wolf wrote: > > Am 21.03.2018 um 15:46 hat Michael S. Tsirkin geschrieben: > > > Our current scheme is to use > > > #include "" > > > for internal headers, and > > > #include <> > > > for external ones. > > > > > > Unfortunately this is not based on compiler support: from C point of > > > view, the "" form merely looks up headers in the current directory > > > and then falls back on <> directories. > > > > > > Thus, for example, a system header trace.h - should it be present - will > > > conflict with our local trace.h > > > > You're right that there is a conflict, even though only in one > > direction: "trace.h" is unambiguously the local trace.h in our source > > tree, but refers to the same local header rather than the > > system header as you would expect. > > > > An easy way to resolve this conflict would be using -iquote rather than > > -I for directories in the source tree, so that unambiguously > > refers to the system header and "trace.h" unambiguously refers to the > > QEMU header. > > I posted patches to that effect for 2.12. Ah, I missed those. That's good (and imho enough). > It's all still very much a non-standard convention and so less robust > than prefixing file name with a project-specifix prefix. I've always had the impression that it's by far the most common convention, to the point that I'd blindly assume it when joining a new project. > > > As another example of problems, a header by the same name in the source > > > directory will always be picked up first - before any headers in > > > the include directory. > > > > > > Let's change the scheme: make sure all headers that are not > > > in the source directory are included through a path > > > starting with qemu/ , thus: > > > > > > #include <> > > > > > > headers in the same directory as source are included with > > > > > > #include "" > > > > > > as per standard. > > > > > > This (untested) patch is just to start the discussion and does not > > > change all of the codebase. If there's agreement, this will be > > > run on all code to converting code to this scheme. > > > > Renaming files is always painful. If that's the fix, the cure might be > > worse than the disease. As far as I know, the conflict is only > > theoretical, so in that case I'd say: If it ain't broke, don't fix it. > > > > Kevin > > It's broke I think, it's very hard for new people to contribute to QEMU. > Look e.g. at rdma which all has messed up includes - and that's from an > experienced conributor who just isn't an experienced maintainer. I don't think the problem is that the convention is hard to apply (it's definitely not). It's knowing about the convention. This problem isn't going away by switching to a different, less common convention. We're only going to see more offenders then. > Amount of time spent on teaching new people trivia about our > conventions just isn't funny. They should be self-documenting > and violations should cause the build to fail. Yes, but your proposal doesn't achieve this. You can still use "qemu/foo.h" instead of and it will build successfully. That's something we can't change, as far as I know, because the include path for "foo.h" is always a superset of . If anything, this means that we should prefer "foo.h" for local headers (i.e. the way it currently is) because we can let the compiler enforce it: for "foo.h" can become a build error, and does so with your -iquote patch, but the other way round doesn't work. Then it's only system headers that you can possibly get wrong, but for those everyone should be used to using anyway. Kevin