From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45925) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ez5ti-0000iI-KA for qemu-devel@nongnu.org; Thu, 22 Mar 2018 15:29:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ez5th-0007LZ-FL for qemu-devel@nongnu.org; Thu, 22 Mar 2018 15:29:14 -0400 Date: Thu, 22 Mar 2018 21:29:00 +0200 From: "Michael S. Tsirkin" Message-ID: <20180322211902-mutt-send-email-mst@kernel.org> References: <1521642402-197739-1-git-send-email-mst@redhat.com> <20180321153439.GC3898@localhost.localdomain> <20180321175452-mutt-send-email-mst@kernel.org> <20180321162203.GE3898@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180321162203.GE3898@localhost.localdomain> Subject: Re: [Qemu-devel] [PATCH v2] qemu: replace "" with <> in headers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf 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?= On Wed, Mar 21, 2018 at 05:22:03PM +0100, Kevin Wolf wrote: > > 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. Any examples? > > > > 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. Not if we have some automatic tools to catch violators. > > 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 the rule is that "" is only for files in the current directory then we can easily code up a checkpatch script to catch violators. > 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 If my proposal to prefix all include directories with qemu/ is accepted, then we can solve the stale file problem by prohibiting a directory named qemu everywhere in source. -- MST