Re: [Qemu-block] [PATCH v2] qemu: replace "" with <> in headers

[Michael S. Tsirkin]

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 <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 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: <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

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