qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] Clean up includes


From: BALATON Zoltan
Subject: Re: [Qemu-devel] [PATCH] Clean up includes
Date: Wed, 13 Mar 2019 21:53:45 +0100 (CET)
User-agent: Alpine 2.21.9999 (BSF 287 2018-06-16)

On Wed, 13 Mar 2019, Markus Armbruster wrote:
BALATON Zoltan <address@hidden> writes:
On Wed, 13 Mar 2019, Markus Armbruster wrote:
Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.

This commit was created with scripts/clean-includes, with the changes
to the following files manually reverted:

   contrib/libvhost-user/libvhost-user-glib.h
   contrib/libvhost-user/libvhost-user.c
   contrib/libvhost-user/libvhost-user.h
   linux-user/mips64/cpu_loop.c
   linux-user/mips64/signal.c
   linux-user/sparc64/cpu_loop.c
   linux-user/sparc64/signal.c
   linux-user/x86_64/cpu_loop.c
   linux-user/x86_64/signal.c
   slirp/src/*
   target/s390x/gen-features.c
   tests/migration/s390x/a-b-bios.c
   tests/test-rcu-simpleq.c
   tests/test-rcu-tailq.c
   tests/uefi-test-tools/UefiTestToolsPkg/BiosTablesTest/BiosTablesTest.c

We're in the process of spinning out slirp/.  tests/uefi-test-tools/
is guest software.  The remaining reverts are the same as in commit
b7d89466dde.

Signed-off-by: Markus Armbruster <address@hidden>
[...]
diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index bc98ba6eeb..f31b3c27c7 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -7,6 +7,7 @@
 * This work is licensed under the GNU GPL license version 2 or later.
 */

+#include "qemu/osdep.h"
#include "ati_int.h"
#include "ati_regs.h"
#include "qemu/log.h"
diff --git a/hw/display/ati_dbg.c b/hw/display/ati_dbg.c
index 1e6c32624e..b045f81d06 100644
--- a/hw/display/ati_dbg.c
+++ b/hw/display/ati_dbg.c
@@ -1,3 +1,4 @@
+#include "qemu/osdep.h"
#include "ati_int.h"

#ifdef DEBUG_ATI
diff --git a/hw/display/ati_int.h b/hw/display/ati_int.h
index a6f3e20e63..2f426064cf 100644
--- a/hw/display/ati_int.h
+++ b/hw/display/ati_int.h
@@ -9,7 +9,6 @@
#ifndef ATI_INT_H
#define ATI_INT_H

-#include "qemu/osdep.h"

What's wrong with ati_int.h including osdep.h first and everything
else including ati_int.h first? I think it was OK that way so unless
there's a good reason to explicitely include osdep in all files that
also include ati_int.h I think these should not be changed. For the
ati model we need ati_int.h included first so it's OK to include
osdep.h from there.

./HACKING explains:

   1.2. Include directives

   Order include directives as follows:

   #include "qemu/osdep.h"  /* Always first... */
   #include <...>           /* then system headers... */
   #include "..."           /* and finally QEMU headers. */

   The "qemu/osdep.h" header contains preprocessor macros that affect the 
behavior
   of core system headers like <stdint.h>.  It must be the first include so that
   core system headers included by external libraries get the preprocessor 
macros
   that QEMU depends on.

   Do not include "qemu/osdep.h" from header files since the .c file will have
   already included it.

I'm not convinced. The rule is to include osdep.h first and it's currently satisified without this change as well but if it makes clean-includes script happy then I don't mind.

Regards,
BALATON Zoltan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]