From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44847) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eyGdg-0005pP-8c for qemu-devel@nongnu.org; Tue, 20 Mar 2018 08:45:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eyGde-00030Z-KO for qemu-devel@nongnu.org; Tue, 20 Mar 2018 08:45:16 -0400 Date: Tue, 20 Mar 2018 14:44:34 +0200 From: "Michael S. Tsirkin" Message-ID: <20180320143945-mutt-send-email-mst@kernel.org> References: <1521510562-529051-1-git-send-email-mst@redhat.com> <8f06e1e3-4823-d8e8-43bd-c092c523849e@vivier.eu> <20180320094406.GD4530@redhat.com> <20180320141107-mutt-send-email-mst@kernel.org> <20180320121841.GN4530@redhat.com> <20180320142756-mutt-send-email-mst@kernel.org> <20180320123900.GO4530@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20180320123900.GO4530@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] qemu: include generated files with <> and not "" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Daniel =?iso-8859-1?Q?P=2E_Berrang=E9?= Cc: Laurent Vivier , qemu-devel@nongnu.org, Paolo Bonzini , Peter Crosthwaite , Richard Henderson , Gerd Hoffmann , Eduardo Habkost , Igor Mammedov , Kevin Wolf , Max Reitz , Jeff Cody , Fam Zheng , John Snow , Stefan Weil , Stefan Hajnoczi , Ronnie Sahlberg , Peter Lieven , Eric Blake , Markus Armbruster , Alberto Garcia , Josh Durgin , Hitoshi Mitake , Liu Yuan , "Richard W.M. Jones" , =?iso-8859-1?Q?Marc-Andr=E9?= Lureau , "Dr. David Alan Gilbert" , Greg Kurz , Ben Warren , Peter Maydell , Shannon Zhao , Michael Walle , Keith Busch , Stefano Stabellini , Anthony Perard , Fabien Chouteau , Amit Shah , Marcel Apfelbaum , Mark Cave-Ayland , BALATON Zoltan , Alexander Graf , Cornelia Huck , Christian Borntraeger , David Gibson , Corey Minyard , =?iso-8859-1?Q?Herv=E9?= Poussineau , Peter Chubb , Subbaraya Sundeep , Dmitry Fleytman , Jason Wang , Yongbok Kim , Max Filippov , Jiri Pirko , Yuval Shaia , David Hildenbrand , Hannes Reinecke , Philippe =?iso-8859-1?Q?Mathieu-Daud=E9?= , Andrzej Zaborowski , Artyom Tarasenko , Alistair Francis , "Edgar E. Iglesias" , Stefan Berger , Alex Williamson , zhanghailiang , Juan Quintela , Michael Roth , Andreas =?iso-8859-1?Q?F=E4rber?= , Pavel Dovgalyuk , Riku Voipio , Zhang Chen , Li Zhijian , Wen Congyang , Xie Changlong , Marcelo Tosatti , Aurelien Jarno , kvm@vger.kernel.org, qemu-block@nongnu.org, sheepdog@lists.wpkg.org, qemu-arm@nongnu.org, xen-devel@lists.xenproject.org, qemu-ppc@nongnu.org, qemu-s390x@nongnu.org, Thomas Huth On Tue, Mar 20, 2018 at 12:39:00PM +0000, Daniel P. Berrang=E9 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=E9 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=E9 wr= ote: > > > > > On Tue, Mar 20, 2018 at 09:58:23AM +0100, Laurent Vivier wrote: > > > > > > Le 20/03/2018 =E0 02:54, Michael S. Tsirkin a =E9crit=A0: > > > > > > > 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. > > > > > >=20 > > > > > > If you change that, we can have issue when a system include h= as the same > > > > > > name as our local include. With "", system header are t= aken first. > > > > >=20 > > > > > > > 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 means look on include search p= ath. > > > > > >=20 > > > > > > Not exactly, there is the notion of "system header" too. > > > > > >=20 > > > > > > https://gcc.gnu.org/onlinedocs/cpp/Include-Syntax.html > > > > > >=20 > > > > > > #include > > > > > > 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). > > > > > >=20 > > > > > > #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 c= urrent > > > > > > file, then in the quote directories and then the same directo= ries used > > > > > > for . You can prepend directories to the list of quote = directories > > > > > > with the -iquote option. > > > > > >=20 > > > > > > > As generated files are not in the search directory (unless = the build > > > > > > > directory happens to match the source directory), it does n= ot 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. > > > > > >=20 > > > > > > I agree there is a problem with stale files. But linux, for i= nstance, > > > > > > asks for a "make mrproper" to avoid this. > > > > >=20 > > > > > 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 configur= ing with > > > > > builddir !=3D srcdir, and exit with error, telling user to clea= n their > > > > > src dir first. > > > > >=20 > > > > > > > This changes include directives for all generated files, ac= ross 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 mergi= ng. > > > > > > >=20 > > > > > > > Note that should some generated files be missed by this tre= e-wide > > > > > > > refactoring, it isn't a big deal - this merely maintains th= e status quo, > > > > > > > and this can be addressed by a separate patch on top. > > > > > > >=20 > > > > > > > Signed-off-by: Michael S. Tsirkin > > > > > >=20 > > > > > > I think your idea conflicts with what Markus has started to d= o: > > > > >=20 > > > > > 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 "<....">. > > > >=20 > > > > Could you please explain what the advantage of "" is? > > > > It seems to be gone since we moved headers away from > > > > source. > > >=20 > > > We moved *some* headers into the include/ directory tree. > > >=20 > > > I still count 650+ headers which are alongside the .c files. > >=20 > > So for these, we should use "". None of these are generated files th= ough. >=20 > That leads to crazy inconsistent message for developers where 50% of QE= MU > 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 wh= y > Markus submitted the patches to standardize on the use of "" for QEMU > header files, leaving <> for system headers & external dependancies. >=20 > 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. > --=20 > |: https://berrange.com -o- https://www.flickr.com/photos/dberr= ange :| > |: https://libvirt.org -o- https://fstop138.berrange= .com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberr= ange :|