From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [PATCH v2] qemu: replace "" with <> in headers Date: Thu, 22 Mar 2018 21:29:00 +0200 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 Cc: Peter Maydell , Dmitry Fleytman , Pavel Dovgalyuk , Li Zhijian , David Hildenbrand , Stefan Hajnoczi , qemu-devel@nongnu.org, BALATON Zoltan , Keith Busch , Max Filippov , Hannes Reinecke , Gerd Hoffmann , Fam Zheng , Max Reitz , Stefano Stabellini , zhanghailiang , Ben Warren , Stefan Berger , Yongbok Kim , Michael Roth , "Richard W.M. Jones" , Christian Borntraeger , =?iso-8859-1?Q?Herv=E9?= Pouss To: Kevin Wolf Return-path: Content-Disposition: inline In-Reply-To: <20180321162203.GE3898@localhost.localdomain> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-block-bounces+gceqb-qemu-block=m.gmane.org@nongnu.org Sender: "Qemu-block" List-Id: kvm.vger.kernel.org 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