Re: [Qemu-arm] [PATCH] qemu: include generated files with <> and not ""

[Michael S. Tsirkin]

On Tue, Mar 20, 2018 at 12:39:00PM +0000, Daniel P. Berrangé wrote:
> On Tue, Mar 20, 2018 at 02:28:42PM +0200, Michael S. Tsirkin wrote:
> > On Tue, Mar 20, 2018 at 12:18:41PM +0000, Daniel P. Berrangé wrote:
> > > On Tue, Mar 20, 2018 at 02:12:24PM +0200, Michael S. Tsirkin wrote:
> > > > On Tue, Mar 20, 2018 at 09:44:06AM +0000, Daniel P. Berrangé wrote:
> > > > > On Tue, Mar 20, 2018 at 09:58:23AM +0100, Laurent Vivier wrote:
> > > > > > Le 20/03/2018 à 02:54, Michael S. Tsirkin a écrit :
> > > > > > > QEMU coding style at the moment asks for all non-system
> > > > > > > include files to be used with #include "foo.h".
> > > > > > > However this rule actually does not make sense and
> > > > > > > creates issues for when the included file is generated.
> > > > > > 
> > > > > > If you change that, we can have issue when a system include has the 
> > > > > > same
> > > > > > name as our local include. With "<FILE>", system header are taken 
> > > > > > first.
> > > > > 
> > > > > > > In C, include "file" means look in current directory,
> > > > > > > then on include search path. Current directory here
> > > > > > > means the source file directory.
> > > > > > > By comparison include <file> means look on include search path.
> > > > > > 
> > > > > > Not exactly, there is the notion of "system header" too.
> > > > > > 
> > > > > > https://gcc.gnu.org/onlinedocs/cpp/Include-Syntax.html
> > > > > > 
> > > > > > #include <file>
> > > > > > This variant is used for system header files. It searches for a file
> > > > > > named file in a standard list of system directories. You can prepend
> > > > > > directories to this list with the -I option (see Invocation).
> > > > > > 
> > > > > > #include "file"
> > > > > > This variant is used for header files of your own program. It 
> > > > > > searches
> > > > > > for a file named file first in the directory containing the current
> > > > > > file, then in the quote directories and then the same directories 
> > > > > > used
> > > > > > for <file>. You can prepend directories to the list of quote 
> > > > > > directories
> > > > > > with the -iquote option.
> > > > > > 
> > > > > > > As generated files are not in the search directory (unless the 
> > > > > > > build
> > > > > > > directory happens to match the source directory), it does not 
> > > > > > > make sense
> > > > > > > to include them with "" - doing so is merely more work for 
> > > > > > > preprocessor
> > > > > > > and a source or errors if a stale file happens to exist in the 
> > > > > > > source
> > > > > > > directory.
> > > > > > 
> > > > > > I agree there is a problem with stale files. But linux, for 
> > > > > > instance,
> > > > > > asks for a "make mrproper" to avoid this.
> > > > > 
> > > > > We can follow what autoconf does, and add a check to configure to see 
> > > > > if
> > > > > there are generated files left in the source dir, when configuring 
> > > > > with
> > > > > builddir != srcdir, and exit with error, telling user to clean their
> > > > > src dir first.
> > > > > 
> > > > > > > This changes include directives for all generated files, across 
> > > > > > > the
> > > > > > > tree. The idea is to avoid sending a huge amount of email.  But 
> > > > > > > when
> > > > > > > merging, the changes will be split with one commit per file, e.g. 
> > > > > > > for
> > > > > > > ease of bisect in case of build failures, and to ease merging.
> > > > > > > 
> > > > > > > Note that should some generated files be missed by this tree-wide
> > > > > > > refactoring, it isn't a big deal - this merely maintains the 
> > > > > > > status quo,
> > > > > > > and this can be addressed by a separate patch on top.
> > > > > > > 
> > > > > > > Signed-off-by: Michael S. Tsirkin <address@hidden>
> > > > > > 
> > > > > > I think your idea conflicts with what Markus has started to do:
> > > > > 
> > > > > Yes, I don't think we should revert what Markus started.   Both ways 
> > > > > of
> > > > > referencing QEMU headers have downsides, but I think "..." has fewer
> > > > > downsides that "<....">.
> > > > 
> > > > Could you please explain what the advantage of "" is?
> > > > It seems to be gone since we moved headers away from
> > > > source.
> > > 
> > > We moved *some* headers into the include/ directory tree.
> > > 
> > > I still count 650+ headers which are alongside the .c files.
> > 
> > So for these, we should use "".  None of these are generated files though.
> 
> That leads to crazy inconsistent message for developers where 50% of QEMU
> header files must use <> and the other 50% of header files must use "".
> Having a consistent message for developers is one of the key reasons why
> Markus submitted the patches to standardize on the use of "" for QEMU
> header files, leaving <> for system headers & external dependancies.
> 
> Regards,
> Daniel

I guess it's in the eye of the beholder. The simple rule since days of
K&R is that "" looks in the current directory of the source.
Whoever learned C knows this.

So use "" if your header is in the same directory as the source.

This will guarantee that whoever violates the rule will get a patch that
does not build.  Having our own rules without any good technical reason
that are not enforced by the build system just leads to small patches
from new contributors getting up to v23.


> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|