[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2] qemu: replace "" with <> in headers
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 <trace.h> 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 <trace.h> 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 <qemu/foo.h> 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 <foo.h>. 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: <foo.h> 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 <foo.h> anyway. Kevin _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |