qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH RFC 0/5] Baby steps towards saner headers
@ 2016-06-23 16:12 Markus Armbruster
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 1/5] Use #include "..." exactly for our own headers Markus Armbruster
                   ` (4 more replies)
  0 siblings, 5 replies; 25+ messages in thread
From: Markus Armbruster @ 2016-06-23 16:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, mst, pbonzini

Some time ago, we discussed rules for headers, and these were
generally liked:

1. Have a carefully curated header that's included everywhere first.  We
   got that already thanks to Peter: osdep.h.

2. Headers should normally include everything they need beyond osdep.h.
   If exceptions are needed for some reason, they must be documented in
   the header.  If all that's needed from a header is typedefs, put
   those into qemu/typedefs.h instead of including the header.

3. Cyclic inclusion is forbidden.

Message-ID: <87h9g8j57d.fsf@blackfin.pond.sub.org>
http://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html

Trouble is we're not exactly close to obeying 2.  This series
demonstrates a possible path towards obeying it: enforce it in "make
check", except for known-bad headers [PATCH 2].  We start with a large
list of known-bad headers, then whittle it down.  Some sample
whittling in PATCH 4+5.

Opinions?

Markus Armbruster (5):
  Use #include "..." exactly for our own headers
  tests: New make target check-headers
  include/qemu/typedefs.h: Restore alphabetical order
  include: Move typedef qemu_irq to qemu/typedefs.h
  include: Include exec/hwaddr.h where hwaddr is used

 block/iscsi.c                     |   1 -
 crypto/pbkdf-gcrypt.c             |   2 +-
 crypto/pbkdf-nettle.c             |   2 +-
 exec.c                            |   2 +-
 hw/audio/lm4549.h                 |   1 +
 hw/cris/boot.h                    |   2 +
 hw/net/e1000e_core.h              |   7 +
 hw/net/vmware_utils.h             |   1 +
 hw/scsi/mptsas.h                  |   1 +
 hw/scsi/virtio-scsi-dataplane.c   |   2 +-
 hw/scsi/virtio-scsi.c             |   2 +-
 include/disas/disas.h             |   1 +
 include/hw/arm/sharpsl.h          |   3 +
 include/hw/block/fdc.h            |   1 +
 include/hw/char/escc.h            |   2 +
 include/hw/char/pl011.h           |   2 +
 include/hw/char/xilinx_uartlite.h |   2 +
 include/hw/cris/etraxfs_dma.h     |   2 +
 include/hw/empty_slot.h           |   2 +
 include/hw/irq.h                  |   2 -
 include/hw/misc/mips_cmgcr.h      |   1 +
 include/hw/pci-host/apb.h         |   1 +
 include/hw/sparc/sparc32_dma.h    |   2 +
 include/hw/timer/m48t59.h         |   1 +
 include/qemu/typedefs.h           |  28 ++-
 include/sysemu/xen-mapcache.h     |   1 +
 linux-user/flatload.c             |   2 +-
 monitor.c                         |   2 +-
 slirp/bootp.c                     |   2 +-
 slirp/cksum.c                     |   2 +-
 slirp/if.c                        |   2 +-
 slirp/ip_input.c                  |   2 +-
 slirp/ip_output.c                 |   2 +-
 slirp/mbuf.c                      |   2 +-
 slirp/misc.c                      |   5 +-
 slirp/sbuf.c                      |   4 +-
 slirp/socket.c                    |   2 +-
 slirp/tcp_input.c                 |   2 +-
 slirp/tcp_output.c                |   2 +-
 slirp/tcp_subr.c                  |   2 +-
 slirp/tcp_timer.c                 |   2 +-
 slirp/tftp.c                      |   2 +-
 slirp/udp.c                       |   2 +-
 target-arm/arm-powerctl.c         |   4 +-
 target-arm/psci.c                 |   8 +-
 target-ppc/mmu-hash32.h           |   2 +
 target-ppc/translate_init.c       |   2 +-
 target-s390x/cpu.h                |   2 +-
 target-unicore32/softmmu.c        |   2 +-
 tests/Makefile.include            | 415 ++++++++++++++++++++++++++++++++++++++
 tests/postcopy-test.c             |   3 +-
 tests/vhost-user-bridge.c         |   2 -
 tests/vhost-user-test.c           |   2 +-
 util/mmap-alloc.c                 |   3 +-
 util/oslib-posix.c                |   2 +-
 55 files changed, 501 insertions(+), 59 deletions(-)

-- 
2.5.5

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH RFC 1/5] Use #include "..." exactly for our own headers
  2016-06-23 16:12 [Qemu-devel] [PATCH RFC 0/5] Baby steps towards saner headers Markus Armbruster
@ 2016-06-23 16:12 ` Markus Armbruster
  2016-06-23 16:31   ` Peter Maydell
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 2/5] tests: New make target check-headers Markus Armbruster
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 25+ messages in thread
From: Markus Armbruster @ 2016-06-23 16:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, mst, pbonzini

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/iscsi.c                   | 1 -
 crypto/pbkdf-gcrypt.c           | 2 +-
 crypto/pbkdf-nettle.c           | 2 +-
 exec.c                          | 2 +-
 hw/scsi/virtio-scsi-dataplane.c | 2 +-
 hw/scsi/virtio-scsi.c           | 2 +-
 linux-user/flatload.c           | 2 +-
 monitor.c                       | 2 +-
 slirp/bootp.c                   | 2 +-
 slirp/cksum.c                   | 2 +-
 slirp/if.c                      | 2 +-
 slirp/ip_input.c                | 2 +-
 slirp/ip_output.c               | 2 +-
 slirp/mbuf.c                    | 2 +-
 slirp/misc.c                    | 5 ++---
 slirp/sbuf.c                    | 4 ++--
 slirp/socket.c                  | 2 +-
 slirp/tcp_input.c               | 2 +-
 slirp/tcp_output.c              | 2 +-
 slirp/tcp_subr.c                | 2 +-
 slirp/tcp_timer.c               | 2 +-
 slirp/tftp.c                    | 2 +-
 slirp/udp.c                     | 2 +-
 target-arm/arm-powerctl.c       | 4 ++--
 target-arm/psci.c               | 8 ++++----
 target-ppc/translate_init.c     | 2 +-
 target-s390x/cpu.h              | 2 +-
 target-unicore32/softmmu.c      | 2 +-
 tests/postcopy-test.c           | 3 +--
 tests/vhost-user-bridge.c       | 2 --
 tests/vhost-user-test.c         | 2 +-
 util/mmap-alloc.c               | 3 ++-
 util/oslib-posix.c              | 2 +-
 33 files changed, 38 insertions(+), 42 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 7e78ade..74f084ca 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -46,7 +46,6 @@
 
 #ifdef __linux__
 #include <scsi/sg.h>
-#include <block/scsi.h>
 #endif
 
 typedef struct IscsiLun {
diff --git a/crypto/pbkdf-gcrypt.c b/crypto/pbkdf-gcrypt.c
index 997b311..34af3a9 100644
--- a/crypto/pbkdf-gcrypt.c
+++ b/crypto/pbkdf-gcrypt.c
@@ -19,9 +19,9 @@
  */
 
 #include "qemu/osdep.h"
+#include <gcrypt.h>
 #include "qapi/error.h"
 #include "crypto/pbkdf.h"
-#include "gcrypt.h"
 
 bool qcrypto_pbkdf2_supports(QCryptoHashAlgorithm hash)
 {
diff --git a/crypto/pbkdf-nettle.c b/crypto/pbkdf-nettle.c
index db9fc15..d681a60 100644
--- a/crypto/pbkdf-nettle.c
+++ b/crypto/pbkdf-nettle.c
@@ -19,9 +19,9 @@
  */
 
 #include "qemu/osdep.h"
+#include <nettle/pbkdf2.h>
 #include "qapi/error.h"
 #include "crypto/pbkdf.h"
-#include "nettle/pbkdf2.h"
 
 
 bool qcrypto_pbkdf2_supports(QCryptoHashAlgorithm hash)
diff --git a/exec.c b/exec.c
index 0122ef7..011babd 100644
--- a/exec.c
+++ b/exec.c
@@ -36,7 +36,7 @@
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
 #if defined(CONFIG_USER_ONLY)
-#include <qemu.h>
+#include "qemu.h"
 #else /* !CONFIG_USER_ONLY */
 #include "hw/hw.h"
 #include "exec/memory.h"
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index 1a49f1e..64c263f 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -16,7 +16,7 @@
 #include "qemu/error-report.h"
 #include "sysemu/block-backend.h"
 #include <hw/scsi/scsi.h>
-#include <block/scsi.h>
+#include "block/scsi.h"
 #include <hw/virtio/virtio-bus.h>
 #include "hw/virtio/virtio-access.h"
 
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 71d09d3..a41f3e9 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -21,7 +21,7 @@
 #include "qemu/iov.h"
 #include "sysemu/block-backend.h"
 #include <hw/scsi/scsi.h>
-#include <block/scsi.h>
+#include "block/scsi.h"
 #include <hw/virtio/virtio-bus.h>
 #include "hw/virtio/virtio-access.h"
 
diff --git a/linux-user/flatload.c b/linux-user/flatload.c
index 48ad1c5..42d1079 100644
--- a/linux-user/flatload.c
+++ b/linux-user/flatload.c
@@ -37,7 +37,7 @@
 
 #include "qemu.h"
 #include "flat.h"
-#include <target_flat.h>
+#include "target_flat.h"
 
 //#define DEBUG
 
diff --git a/monitor.c b/monitor.c
index 6f960f1..6770c8c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -62,7 +62,7 @@
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/json-streamer.h"
 #include "qapi/qmp/json-parser.h"
-#include <qom/object_interfaces.h>
+#include "qom/object_interfaces.h"
 #include "cpu.h"
 #include "trace.h"
 #include "trace/control.h"
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 7b3232b..5a4646c 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -22,7 +22,7 @@
  * THE SOFTWARE.
  */
 #include "qemu/osdep.h"
-#include <slirp.h>
+#include "slirp.h"
 
 #if defined(_WIN32)
 /* Windows ntohl() returns an u_long value.
diff --git a/slirp/cksum.c b/slirp/cksum.c
index 2ad0e65..6d73abf 100644
--- a/slirp/cksum.c
+++ b/slirp/cksum.c
@@ -31,7 +31,7 @@
  */
 
 #include "qemu/osdep.h"
-#include <slirp.h>
+#include "slirp.h"
 
 /*
  * Checksum routine for Internet Protocol family headers (Portable Version).
diff --git a/slirp/if.c b/slirp/if.c
index 9b02180..51ae0d0 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -6,7 +6,7 @@
  */
 
 #include "qemu/osdep.h"
-#include <slirp.h>
+#include "slirp.h"
 #include "qemu/timer.h"
 
 static void
diff --git a/slirp/ip_input.c b/slirp/ip_input.c
index 34fba2b..348e1dc 100644
--- a/slirp/ip_input.c
+++ b/slirp/ip_input.c
@@ -39,7 +39,7 @@
  */
 
 #include "qemu/osdep.h"
-#include <slirp.h>
+#include "slirp.h"
 #include "ip_icmp.h"
 
 static struct ip *ip_reass(Slirp *slirp, struct ip *ip, struct ipq *fp);
diff --git a/slirp/ip_output.c b/slirp/ip_output.c
index 0d6b3b8..db403f0 100644
--- a/slirp/ip_output.c
+++ b/slirp/ip_output.c
@@ -39,7 +39,7 @@
  */
 
 #include "qemu/osdep.h"
-#include <slirp.h>
+#include "slirp.h"
 
 /* Number of packets queued before we start sending
  * (to prevent allocing too many mbufs) */
diff --git a/slirp/mbuf.c b/slirp/mbuf.c
index d136988..7eddc21 100644
--- a/slirp/mbuf.c
+++ b/slirp/mbuf.c
@@ -16,7 +16,7 @@
  */
 
 #include "qemu/osdep.h"
-#include <slirp.h>
+#include "slirp.h"
 
 #define MBUF_THRESH 30
 
diff --git a/slirp/misc.c b/slirp/misc.c
index 1a0ea1b..88e9d94 100644
--- a/slirp/misc.c
+++ b/slirp/misc.c
@@ -6,9 +6,8 @@
  */
 
 #include "qemu/osdep.h"
-#include <slirp.h>
-#include <libslirp.h>
-
+#include "slirp.h"
+#include "libslirp.h"
 #include "monitor/monitor.h"
 #include "qemu/error-report.h"
 #include "qemu/main-loop.h"
diff --git a/slirp/sbuf.c b/slirp/sbuf.c
index dd4cb8c..10119d3 100644
--- a/slirp/sbuf.c
+++ b/slirp/sbuf.c
@@ -6,8 +6,8 @@
  */
 
 #include "qemu/osdep.h"
-#include <slirp.h>
-#include <qemu/main-loop.h>
+#include "slirp.h"
+#include "qemu/main-loop.h"
 
 static void sbappendsb(struct sbuf *sb, struct mbuf *m);
 
diff --git a/slirp/socket.c b/slirp/socket.c
index b336586..2b53cd7 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -7,7 +7,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu-common.h"
-#include <slirp.h>
+#include "slirp.h"
 #include "ip_icmp.h"
 #ifdef __sun__
 #include <sys/filio.h>
diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c
index e2b5d4e..c5063a9 100644
--- a/slirp/tcp_input.c
+++ b/slirp/tcp_input.c
@@ -39,7 +39,7 @@
  */
 
 #include "qemu/osdep.h"
-#include <slirp.h>
+#include "slirp.h"
 #include "ip_icmp.h"
 
 #define	TCPREXMTTHRESH 3
diff --git a/slirp/tcp_output.c b/slirp/tcp_output.c
index 99b0a9b..819db27 100644
--- a/slirp/tcp_output.c
+++ b/slirp/tcp_output.c
@@ -39,7 +39,7 @@
  */
 
 #include "qemu/osdep.h"
-#include <slirp.h>
+#include "slirp.h"
 
 static const u_char  tcp_outflags[TCP_NSTATES] = {
 	TH_RST|TH_ACK, 0,      TH_SYN,        TH_SYN|TH_ACK,
diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index 6b9fef2..ed16e18 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -39,7 +39,7 @@
  */
 
 #include "qemu/osdep.h"
-#include <slirp.h>
+#include "slirp.h"
 
 /* patchable/settable parameters for tcp */
 /* Don't do rfc1323 performance enhancements */
diff --git a/slirp/tcp_timer.c b/slirp/tcp_timer.c
index 8f5dd77..f9060c7 100644
--- a/slirp/tcp_timer.c
+++ b/slirp/tcp_timer.c
@@ -31,7 +31,7 @@
  */
 
 #include "qemu/osdep.h"
-#include <slirp.h>
+#include "slirp.h"
 
 static struct tcpcb *tcp_timers(register struct tcpcb *tp, int timer);
 
diff --git a/slirp/tftp.c b/slirp/tftp.c
index 12b5ff6..8de3bc0 100644
--- a/slirp/tftp.c
+++ b/slirp/tftp.c
@@ -23,7 +23,7 @@
  */
 
 #include "qemu/osdep.h"
-#include <slirp.h>
+#include "slirp.h"
 #include "qemu-common.h"
 #include "qemu/cutils.h"
 
diff --git a/slirp/udp.c b/slirp/udp.c
index 247024f..93d7224 100644
--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -39,7 +39,7 @@
  */
 
 #include "qemu/osdep.h"
-#include <slirp.h>
+#include "slirp.h"
 #include "ip_icmp.h"
 
 static uint8_t udp_tos(struct socket *so);
diff --git a/target-arm/arm-powerctl.c b/target-arm/arm-powerctl.c
index d452230..6519d52 100644
--- a/target-arm/arm-powerctl.c
+++ b/target-arm/arm-powerctl.c
@@ -9,8 +9,8 @@
  */
 
 #include "qemu/osdep.h"
-#include <cpu.h>
-#include <cpu-qom.h>
+#include "cpu.h"
+#include "cpu-qom.h"
 #include "internals.h"
 #include "arm-powerctl.h"
 #include "qemu/log.h"
diff --git a/target-arm/psci.c b/target-arm/psci.c
index 4db9b8c..14316eb 100644
--- a/target-arm/psci.c
+++ b/target-arm/psci.c
@@ -16,10 +16,10 @@
  *  along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 #include "qemu/osdep.h"
-#include <cpu.h>
-#include <exec/helper-proto.h>
-#include <kvm-consts.h>
-#include <sysemu/sysemu.h>
+#include "cpu.h"
+#include "exec/helper-proto.h"
+#include "kvm-consts.h"
+#include "sysemu/sysemu.h"
 #include "internals.h"
 #include "arm-powerctl.h"
 #include "exec/exec-all.h"
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index ca894ff..49c0728 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -21,7 +21,7 @@
 #include "qemu/osdep.h"
 #include "disas/bfd.h"
 #include "exec/gdbstub.h"
-#include <sysemu/kvm.h>
+#include "sysemu/kvm.h"
 #include "kvm_ppc.h"
 #include "sysemu/arch_init.h"
 #include "sysemu/cpus.h"
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index bd6b2e5..c7cc4e1 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -219,7 +219,7 @@ int s390_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 void s390_cpu_gdb_init(CPUState *cs);
 void s390x_cpu_debug_excp_handler(CPUState *cs);
 
-#include <sysemu/kvm.h>
+#include "sysemu/kvm.h"
 
 /* distinguish between 24 bit and 31 bit addressing */
 #define HIGH_ORDER_BIT 0x80000000
diff --git a/target-unicore32/softmmu.c b/target-unicore32/softmmu.c
index a34026a..e7152e7 100644
--- a/target-unicore32/softmmu.c
+++ b/target-unicore32/softmmu.c
@@ -13,7 +13,7 @@
 #endif
 
 #include "qemu/osdep.h"
-#include <cpu.h>
+#include "cpu.h"
 #include "exec/exec-all.h"
 
 #undef DEBUG_UC32
diff --git a/tests/postcopy-test.c b/tests/postcopy-test.c
index 35d5180..16465ab 100644
--- a/tests/postcopy-test.c
+++ b/tests/postcopy-test.c
@@ -15,11 +15,10 @@
 #include "libqtest.h"
 #include "qemu/option.h"
 #include "qemu/range.h"
+#include "qemu/sockets.h"
 #include "sysemu/char.h"
 #include "sysemu/sysemu.h"
 
-#include <qemu/sockets.h>
-
 const unsigned start_address = 1024 * 1024;
 const unsigned end_address = 100 * 1024 * 1024;
 bool got_stop;
diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
index 45fa2b6..775e031 100644
--- a/tests/vhost-user-bridge.c
+++ b/tests/vhost-user-bridge.c
@@ -36,8 +36,6 @@
 #include <sys/eventfd.h>
 #include <arpa/inet.h>
 #include <netdb.h>
-#include <qemu/osdep.h>
-
 #include <linux/vhost.h>
 
 #include "qemu/atomic.h"
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 8b2164b..71e7f42 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -13,12 +13,12 @@
 #include "libqtest.h"
 #include "qemu/option.h"
 #include "qemu/range.h"
+#include "qemu/sockets.h"
 #include "sysemu/char.h"
 #include "sysemu/sysemu.h"
 
 #include <linux/vhost.h>
 #include <sys/vfs.h>
-#include <qemu/sockets.h>
 
 /* GLIB version compatibility flags */
 #if !GLIB_CHECK_VERSION(2, 26, 0)
diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
index 629d97a..5a85aa3 100644
--- a/util/mmap-alloc.c
+++ b/util/mmap-alloc.c
@@ -9,8 +9,9 @@
  * This work is licensed under the terms of the GNU GPL, version 2 or
  * later.  See the COPYING file in the top-level directory.
  */
+
 #include "qemu/osdep.h"
-#include <qemu/mmap-alloc.h>
+#include "qemu/mmap-alloc.h"
 
 #define HUGETLBFS_MAGIC       0x958458f6
 
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index e2e1d4d..d8e5dcf 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -48,7 +48,7 @@
 #include <sys/sysctl.h>
 #endif
 
-#include <qemu/mmap-alloc.h>
+#include "qemu/mmap-alloc.h"
 
 int qemu_get_thread_id(void)
 {
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH RFC 2/5] tests: New make target check-headers
  2016-06-23 16:12 [Qemu-devel] [PATCH RFC 0/5] Baby steps towards saner headers Markus Armbruster
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 1/5] Use #include "..." exactly for our own headers Markus Armbruster
@ 2016-06-23 16:12 ` Markus Armbruster
  2016-06-23 16:40   ` Paolo Bonzini
  2016-06-23 19:33   ` Eric Blake
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 3/5] include/qemu/typedefs.h: Restore alphabetical order Markus Armbruster
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 25+ messages in thread
From: Markus Armbruster @ 2016-06-23 16:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, mst, pbonzini

For each header "FOO.h", the test checks whether

	#include "qemu/osdep.h"
	#include "FOO.h"
	#include "FOO.h"

compiles.  A large number of headers don't pass this test, by design
or by accident.  These are all excluded with a blacklist for now.  Add
make target check-blacklisted-headers to help with examinating how
they fail.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/Makefile.include | 426 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 426 insertions(+)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index a2ed83b..e20f437 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -413,6 +413,421 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
 $(test-obj-y): QEMU_INCLUDES += -Itests
 QEMU_CFLAGS += -I$(SRC_PATH)/tests
 
+# All headers
+headers := $(filter %.h, $(shell cd $(SRC_PATH) && git ls-files) $(GENERATED_HEADERS))
+# Headers that need more than just #include "qemu/osdep.h" to compile
+blacklisted-headers :=					\
+	audio/audio_int.h				\
+	audio/audio_template.h				\
+	audio/audio_win_int.h				\
+	audio/dsound_template.h				\
+	audio/mixeng_template.h				\
+	audio/rate_template.h				\
+	block/qcow2.h					\
+	block/raw-aio.h					\
+	block/vhdx.h					\
+	bsd-user/i386/target_signal.h			\
+	bsd-user/i386/target_syscall.h			\
+	bsd-user/qemu.h					\
+	bsd-user/sparc/target_signal.h			\
+	bsd-user/sparc/target_syscall.h			\
+	bsd-user/sparc64/target_signal.h		\
+	bsd-user/sparc64/target_syscall.h		\
+	bsd-user/syscall_defs.h				\
+	bsd-user/x86_64/target_signal.h			\
+	bsd-user/x86_64/target_syscall.h		\
+	disas/libvixl/vixl/a64/assembler-a64.h		\
+	disas/libvixl/vixl/a64/constants-a64.h		\
+	disas/libvixl/vixl/a64/cpu-a64.h		\
+	disas/libvixl/vixl/a64/decoder-a64.h		\
+	disas/libvixl/vixl/a64/disasm-a64.h		\
+	disas/libvixl/vixl/a64/instructions-a64.h	\
+	disas/libvixl/vixl/code-buffer.h		\
+	disas/libvixl/vixl/compiler-intrinsics.h	\
+	disas/libvixl/vixl/globals.h			\
+	disas/libvixl/vixl/invalset.h			\
+	disas/libvixl/vixl/platform.h			\
+	disas/libvixl/vixl/utils.h			\
+	fpu/softfloat-macros.h				\
+	fpu/softfloat-specialize.h			\
+	hw/9pfs/9p-synth.h				\
+	hw/9pfs/9p-xattr.h				\
+	hw/audio/fmopl.h				\
+	hw/audio/hda-codec-common.h			\
+	hw/audio/lm4549.h				\
+	hw/block/nvme.h					\
+	hw/block/xen_blkif.h				\
+	hw/cris/boot.h					\
+	hw/display/cirrus_vga_rop.h			\
+	hw/display/cirrus_vga_rop2.h			\
+	hw/display/milkymist-vgafb_template.h		\
+	hw/display/omap_lcd_template.h			\
+	hw/display/pl110_template.h			\
+	hw/display/pxa2xx_template.h			\
+	hw/display/sm501_template.h			\
+	hw/display/tc6393xb_template.h			\
+	hw/display/vga-helpers.h			\
+	hw/display/vga_int.h				\
+	hw/i386/intel_iommu_internal.h			\
+	hw/ide/ahci.h					\
+	hw/lm32/lm32.h					\
+	hw/lm32/lm32_hwsetup.h				\
+	hw/lm32/milkymist-hw.h				\
+	hw/microblaze/boot.h				\
+	hw/net/e1000e_core.h				\
+	hw/net/e1000x_common.h				\
+	hw/net/ne2000.h					\
+	hw/net/pcnet.h					\
+	hw/net/rocker/rocker_desc.h			\
+	hw/net/rocker/rocker_fp.h			\
+	hw/net/rocker/rocker_of_dpa.h			\
+	hw/net/rocker/rocker_tlv.h			\
+	hw/net/rocker/rocker_world.h			\
+	hw/net/vmware_utils.h				\
+	hw/ppc/ppc405.h					\
+	hw/s390x/ipl.h					\
+	hw/s390x/s390-pci-inst.h			\
+	hw/s390x/s390-virtio.h				\
+	hw/scsi/mptsas.h				\
+	hw/scsi/viosrp.h				\
+	hw/tpm/tpm_int.h				\
+	hw/tpm/tpm_tis.h				\
+	hw/usb/desc.h					\
+	hw/usb/quirks.h					\
+	hw/xtensa/bootparam.h				\
+	include/block/write-threshold.h			\
+	include/disas/disas.h				\
+	include/exec/cpu-all.h				\
+	include/exec/cpu-defs.h				\
+	include/exec/cpu_ldst.h				\
+	include/exec/cpu_ldst_template.h		\
+	include/exec/cpu_ldst_useronly_template.h	\
+	include/exec/cputlb.h				\
+	include/exec/exec-all.h				\
+	include/exec/gen-icount.h			\
+	include/exec/helper-gen.h			\
+	include/exec/helper-proto.h			\
+	include/exec/helper-tcg.h			\
+	include/exec/ioport.h				\
+	include/exec/memory-internal.h			\
+	include/exec/ram_addr.h				\
+	include/exec/softmmu-semi.h			\
+	include/exec/tb-hash.h				\
+	include/exec/user/abitypes.h			\
+	include/exec/user/thunk.h			\
+	include/hw/acpi/piix4.h				\
+	include/hw/acpi/tco.h				\
+	include/hw/arm/allwinner-a10.h			\
+	include/hw/arm/bcm2836.h			\
+	include/hw/arm/digic.h				\
+	include/hw/arm/fsl-imx25.h			\
+	include/hw/arm/fsl-imx31.h			\
+	include/hw/arm/fsl-imx6.h			\
+	include/hw/arm/sharpsl.h			\
+	include/hw/arm/xlnx-zynqmp.h			\
+	include/hw/block/fdc.h				\
+	include/hw/block/flash.h			\
+	include/hw/char/escc.h				\
+	include/hw/char/pl011.h				\
+	include/hw/char/xilinx_uartlite.h		\
+	include/hw/cris/etraxfs.h			\
+	include/hw/cris/etraxfs_dma.h			\
+	include/hw/elf_ops.h				\
+	include/hw/empty_slot.h				\
+	include/hw/i2c/aspeed_i2c.h			\
+	include/hw/i2c/i2c-ddc.h			\
+	include/hw/i2c/pm_smbus.h			\
+	include/hw/i386/apic_internal.h			\
+	include/hw/i386/intel_iommu.h			\
+	include/hw/i386/ioapic_internal.h		\
+	include/hw/input/hid.h				\
+	include/hw/intc/allwinner-a10-pic.h		\
+	include/hw/isa/i8257.h				\
+	include/hw/isa/vt82c686.h			\
+	include/hw/kvm/clock.h				\
+	include/hw/mips/bios.h				\
+	include/hw/mips/cpudevs.h			\
+	include/hw/mips/mips.h				\
+	include/hw/misc/mips_cmgcr.h			\
+	include/hw/misc/mips_cpc.h			\
+	include/hw/misc/mips_itu.h			\
+	include/hw/net/allwinner_emac.h			\
+	include/hw/nvram/openbios_firmware_abi.h	\
+	include/hw/pci-host/apb.h			\
+	include/hw/pci-host/spapr.h			\
+	include/hw/pci/pci_bus.h			\
+	include/hw/pci/pcie_aer.h			\
+	include/hw/ppc/ppc.h				\
+	include/hw/ppc/ppc4xx.h				\
+	include/hw/ppc/spapr.h				\
+	include/hw/ppc/spapr_cpu_core.h			\
+	include/hw/ppc/spapr_vio.h			\
+	include/hw/ppc/xics.h				\
+	include/hw/sparc/grlib.h			\
+	include/hw/sparc/sparc32_dma.h			\
+	include/hw/ssi/xilinx_spips.h			\
+	include/hw/timer/allwinner-a10-pit.h		\
+	include/hw/timer/aspeed_timer.h			\
+	include/hw/timer/i8254_internal.h		\
+	include/hw/timer/m48t59.h			\
+	include/hw/virtio/virtio-access.h		\
+	include/hw/virtio/virtio-input.h		\
+	include/hw/virtio/virtio-rng.h			\
+	include/libdecnumber/decNumberLocal.h		\
+	include/migration/cpu.h				\
+	include/monitor/hmp-target.h			\
+	include/qemu/ratelimit.h			\
+	include/qemu/thread-win32.h			\
+	include/sysemu/balloon.h			\
+	include/sysemu/cpus.h				\
+	include/sysemu/dump.h				\
+	include/sysemu/iothread.h			\
+	include/sysemu/kvm_int.h			\
+	include/sysemu/memory_mapping.h			\
+	include/sysemu/os-win32.h			\
+	include/sysemu/tpm.h				\
+	include/sysemu/xen-mapcache.h			\
+	include/trace-tcg.h				\
+	include/ui/egl-context.h			\
+	include/ui/egl-helpers.h			\
+	include/ui/gtk.h				\
+	include/ui/input.h				\
+	include/ui/pixel_ops.h				\
+	include/ui/sdl2.h				\
+	include/ui/spice-display.h			\
+	linux-headers/asm-arm64/kvm.h			\
+	linux-headers/asm-mips/unistd.h			\
+	linux-headers/asm-powerpc/kvm_para.h		\
+	linux-user/aarch64/target_cpu.h			\
+	linux-user/aarch64/target_signal.h		\
+	linux-user/aarch64/target_structs.h		\
+	linux-user/aarch64/termbits.h			\
+	linux-user/alpha/target_cpu.h			\
+	linux-user/alpha/target_signal.h		\
+	linux-user/alpha/target_structs.h		\
+	linux-user/alpha/target_syscall.h		\
+	linux-user/alpha/termbits.h			\
+	linux-user/arm/nwfpe/fpa11.h			\
+	linux-user/arm/nwfpe/fpopcode.h			\
+	linux-user/arm/target_cpu.h			\
+	linux-user/arm/target_signal.h			\
+	linux-user/arm/target_structs.h			\
+	linux-user/arm/target_syscall.h			\
+	linux-user/arm/termbits.h			\
+	linux-user/cris/target_cpu.h			\
+	linux-user/cris/target_signal.h			\
+	linux-user/cris/target_structs.h		\
+	linux-user/cris/termbits.h			\
+	linux-user/flat.h				\
+	linux-user/i386/target_cpu.h			\
+	linux-user/i386/target_signal.h			\
+	linux-user/i386/target_structs.h		\
+	linux-user/i386/target_syscall.h		\
+	linux-user/i386/termbits.h			\
+	linux-user/ioctls.h				\
+	linux-user/m68k/target_cpu.h			\
+	linux-user/m68k/target_signal.h			\
+	linux-user/m68k/target_structs.h		\
+	linux-user/m68k/target_syscall.h		\
+	linux-user/m68k/termbits.h			\
+	linux-user/microblaze/target_cpu.h		\
+	linux-user/microblaze/target_signal.h		\
+	linux-user/microblaze/target_structs.h		\
+	linux-user/microblaze/termbits.h		\
+	linux-user/mips/target_cpu.h			\
+	linux-user/mips/target_signal.h			\
+	linux-user/mips/target_structs.h		\
+	linux-user/mips/target_syscall.h		\
+	linux-user/mips/termbits.h			\
+	linux-user/mips64/syscall_nr.h			\
+	linux-user/mips64/target_cpu.h			\
+	linux-user/mips64/target_signal.h		\
+	linux-user/mips64/target_structs.h		\
+	linux-user/mips64/target_syscall.h		\
+	linux-user/mips64/termbits.h			\
+	linux-user/openrisc/target_cpu.h		\
+	linux-user/openrisc/target_signal.h		\
+	linux-user/openrisc/target_structs.h		\
+	linux-user/openrisc/target_syscall.h		\
+	linux-user/openrisc/termbits.h			\
+	linux-user/ppc/syscall_nr.h			\
+	linux-user/ppc/target_cpu.h			\
+	linux-user/ppc/target_signal.h			\
+	linux-user/ppc/target_structs.h			\
+	linux-user/ppc/target_syscall.h			\
+	linux-user/ppc/termbits.h			\
+	linux-user/qemu.h				\
+	linux-user/s390x/target_cpu.h			\
+	linux-user/s390x/target_signal.h		\
+	linux-user/s390x/target_structs.h		\
+	linux-user/s390x/target_syscall.h		\
+	linux-user/s390x/termbits.h			\
+	linux-user/sh4/target_cpu.h			\
+	linux-user/sh4/target_signal.h			\
+	linux-user/sh4/target_structs.h			\
+	linux-user/sh4/termbits.h			\
+	linux-user/socket.h				\
+	linux-user/sparc/target_cpu.h			\
+	linux-user/sparc/target_signal.h		\
+	linux-user/sparc/target_structs.h		\
+	linux-user/sparc/target_syscall.h		\
+	linux-user/sparc/termbits.h			\
+	linux-user/sparc64/target_cpu.h			\
+	linux-user/sparc64/target_signal.h		\
+	linux-user/sparc64/target_structs.h		\
+	linux-user/sparc64/target_syscall.h		\
+	linux-user/sparc64/termbits.h			\
+	linux-user/syscall_defs.h			\
+	linux-user/syscall_types.h			\
+	linux-user/tilegx/target_cpu.h			\
+	linux-user/tilegx/target_signal.h		\
+	linux-user/tilegx/target_structs.h		\
+	linux-user/tilegx/target_syscall.h		\
+	linux-user/unicore32/target_cpu.h		\
+	linux-user/unicore32/target_signal.h		\
+	linux-user/unicore32/target_structs.h		\
+	linux-user/unicore32/target_syscall.h		\
+	linux-user/unicore32/termbits.h			\
+	linux-user/x86_64/target_cpu.h			\
+	linux-user/x86_64/target_signal.h		\
+	linux-user/x86_64/target_structs.h		\
+	linux-user/x86_64/target_syscall.h		\
+	linux-user/x86_64/termbits.h			\
+	pc-bios/optionrom/optionrom.h			\
+	pc-bios/s390-ccw/bootmap.h			\
+	pc-bios/s390-ccw/cio.h				\
+	pc-bios/s390-ccw/iplb.h				\
+	pc-bios/s390-ccw/s390-ccw.h			\
+	pc-bios/s390-ccw/scsi.h				\
+	pc-bios/s390-ccw/virtio-scsi.h			\
+	pc-bios/s390-ccw/virtio.h			\
+	qemu-options-wrapper.h				\
+	qga/guest-agent-core.h				\
+	qga/service-win32.h				\
+	qga/vss-win32/requester.h			\
+	qga/vss-win32/vss-common.h			\
+	replay/replay-internal.h			\
+	scripts/cocci-macro-file.h			\
+	slirp/bootp.h					\
+	slirp/ip.h					\
+	slirp/ip6_icmp.h				\
+	slirp/ip_icmp.h					\
+	slirp/main.h					\
+	slirp/mbuf.h					\
+	slirp/misc.h					\
+	slirp/sbuf.h					\
+	slirp/socket.h					\
+	slirp/tcp.h					\
+	slirp/tcp_timer.h				\
+	slirp/tcp_var.h					\
+	slirp/tcpip.h					\
+	slirp/tftp.h					\
+	slirp/udp.h					\
+	softmmu_template.h				\
+	target-alpha/cpu.h				\
+	target-alpha/helper.h				\
+	target-arm/arm_ldst.h				\
+	target-arm/cpu.h				\
+	target-arm/helper-a64.h				\
+	target-arm/helper.h				\
+	target-arm/internals.h				\
+	target-arm/kvm_arm.h				\
+	target-arm/op_addsub.h				\
+	target-arm/translate.h				\
+	target-cris/cpu.h				\
+	target-cris/helper.h				\
+	target-cris/mmu.h				\
+	target-i386/cc_helper_template.h		\
+	target-i386/cpu-qom.h				\
+	target-i386/cpu.h				\
+	target-i386/helper.h				\
+	target-i386/hyperv.h				\
+	target-i386/kvm_i386.h				\
+	target-i386/ops_sse.h				\
+	target-i386/ops_sse_header.h			\
+	target-i386/shift_helper_template.h		\
+	target-lm32/cpu.h				\
+	target-lm32/helper.h				\
+	target-m68k/cpu.h				\
+	target-m68k/helper.h				\
+	target-microblaze/cpu.h				\
+	target-microblaze/helper.h			\
+	target-microblaze/mmu.h				\
+	target-mips/cpu-qom.h				\
+	target-mips/cpu.h				\
+	target-mips/helper.h				\
+	target-mips/kvm_mips.h				\
+	target-mips/mips-defs.h				\
+	target-moxie/cpu.h				\
+	target-moxie/helper.h				\
+	target-moxie/machine.h				\
+	target-moxie/mmu.h				\
+	target-openrisc/cpu.h				\
+	target-openrisc/exception.h			\
+	target-openrisc/helper.h			\
+	target-ppc/cpu-models.h				\
+	target-ppc/cpu-qom.h				\
+	target-ppc/cpu.h				\
+	target-ppc/helper.h				\
+	target-ppc/helper_regs.h			\
+	target-ppc/kvm_ppc.h				\
+	target-ppc/mmu-hash32.h				\
+	target-ppc/mmu-hash64.h				\
+	target-s390x/cpu.h				\
+	target-s390x/helper.h				\
+	target-sh4/cpu.h				\
+	target-sh4/helper.h				\
+	target-sparc/cpu-qom.h				\
+	target-sparc/cpu.h				\
+	target-sparc/helper.h				\
+	target-tilegx/cpu.h				\
+	target-tilegx/helper.h				\
+	target-tricore/cpu.h				\
+	target-tricore/helper.h				\
+	target-tricore/tricore-defs.h			\
+	target-tricore/tricore-opcodes.h		\
+	target-unicore32/cpu.h				\
+	target-unicore32/helper.h			\
+	target-xtensa/cpu.h				\
+	target-xtensa/helper.h				\
+	target-xtensa/overlay_tool.h			\
+	tcg/mips/tcg-target.h				\
+	tcg/tcg-be-ldst.h				\
+	tcg/tcg-be-null.h				\
+	tcg/tcg-op.h					\
+	tcg/tcg-opc.h					\
+	tcg/tcg-runtime.h				\
+	tcg/tcg.h					\
+	tests/crypto-tls-x509-helpers.h			\
+	tests/multiboot/libc.h				\
+	tests/multiboot/multiboot.h			\
+	tests/tcg/cris/crisutils.h			\
+	tests/tcg/cris/sys.h				\
+	tests/tcg/mips/mips64-dsp/io.h			\
+	tests/tcg/mips/mips64-dspr2/io.h		\
+	tests/tcg/test-i386-muldiv.h			\
+	tests/tcg/test-i386-shift.h			\
+	tests/tcg/test-i386.h				\
+	trace/control-internal.h			\
+	trace/generated-tcg-tracers.h			\
+	trace/mem-internal.h				\
+	trace/mem.h					\
+	trace/simple.h					\
+	translate-all.h					\
+	ui/curses_keys.h				\
+	ui/sdl2-keymap.h				\
+	ui/sdl_keysym.h					\
+	ui/sdl_zoom.h					\
+	ui/sdl_zoom_template.h				\
+	ui/vgafont.h					\
+	ui/vnc-auth-sasl.h				\
+	ui/vnc-auth-vencrypt.h				\
+	ui/vnc-enc-hextile-template.h			\
+	ui/vnc-jobs.h					\
+	ui/vnc-ws.h					\
+	ui/vnc_keysym.h
+checked-headers := $(filter-out $(blacklisted-headers), $(headers))
+-include $(patsubst %.h,tests/headers/%.d, $(checked-headers))
 
 # Deps that are common to various different sets of tests below
 test-util-obj-y = libqemuutil.a libqemustub.a
@@ -647,6 +1062,7 @@ check-help:
 	@echo " make check-unit           Run qobject tests"
 	@echo " make check-qapi-schema    Run QAPI schema tests"
 	@echo " make check-block          Run block tests"
+	@echo " make check-headers        Run header sanity tests"
 	@echo " make check-report.html    Generates an HTML test report"
 	@echo " make check-clean          Clean the tests"
 	@echo
@@ -728,8 +1144,18 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json
 	@perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -q $(SRC_PATH)/$*.err -
 	@diff -q $(SRC_PATH)/$*.exit $*.test.exit
 
+# Header sanity checking
+
+tests/headers/%.c: tests/header-test-template.c
+	@mkdir -p $(dir $@)
+	@sed 's,@header@,$(subst tests/headers/,,$(@:.c=.h)),' <$< >$@
+
 # Consolidated targets
 
+.PHONY: check-headers check-blacklisted-headers
+check-headers: $(patsubst %.h,tests/headers/%.o, $(checked-headers))
+check-blacklisted-headers: $(patsubst %.h,tests/headers/%.o, $(blacklisted-headers))
+
 .PHONY: check-qapi-schema check-qtest check-unit check check-clean
 check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y))
 check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH RFC 3/5] include/qemu/typedefs.h: Restore alphabetical order
  2016-06-23 16:12 [Qemu-devel] [PATCH RFC 0/5] Baby steps towards saner headers Markus Armbruster
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 1/5] Use #include "..." exactly for our own headers Markus Armbruster
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 2/5] tests: New make target check-headers Markus Armbruster
@ 2016-06-23 16:12 ` Markus Armbruster
  2016-06-23 16:40   ` Peter Maydell
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 4/5] include: Move typedef qemu_irq to qemu/typedefs.h Markus Armbruster
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 5/5] include: Include exec/hwaddr.h where hwaddr is used Markus Armbruster
  4 siblings, 1 reply; 25+ messages in thread
From: Markus Armbruster @ 2016-06-23 16:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, mst, pbonzini

While there, drop a comment that has become misleading.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qemu/typedefs.h | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index b113fcf..f9745c4 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -1,9 +1,6 @@
 #ifndef QEMU_TYPEDEFS_H
 #define QEMU_TYPEDEFS_H
 
-/* A load of opaque types so that device init declarations don't have to
-   pull in all the real definitions.  */
-
 /* Please keep this list in alphabetical order */
 typedef struct AdapterInfo AdapterInfo;
 typedef struct AddressSpace AddressSpace;
@@ -16,10 +13,10 @@ typedef struct BlockBackendRootState BlockBackendRootState;
 typedef struct BlockDriverState BlockDriverState;
 typedef struct BusClass BusClass;
 typedef struct BusState BusState;
-typedef struct CharDriverState CharDriverState;
-typedef struct CompatProperty CompatProperty;
 typedef struct CPUAddressSpace CPUAddressSpace;
 typedef struct CPUState CPUState;
+typedef struct CharDriverState CharDriverState;
+typedef struct CompatProperty CompatProperty;
 typedef struct DeviceListener DeviceListener;
 typedef struct DeviceState DeviceState;
 typedef struct DisplayChangeListener DisplayChangeListener;
@@ -39,6 +36,7 @@ typedef struct ISADevice ISADevice;
 typedef struct IsaDma IsaDma;
 typedef struct LoadStateEntry LoadStateEntry;
 typedef struct MACAddr MACAddr;
+typedef struct MSIMessage MSIMessage;
 typedef struct MachineClass MachineClass;
 typedef struct MachineState MachineState;
 typedef struct MemoryListener MemoryListener;
@@ -51,11 +49,9 @@ typedef struct MigrationState MigrationState;
 typedef struct Monitor Monitor;
 typedef struct MonitorDef MonitorDef;
 typedef struct MouseTransformInfo MouseTransformInfo;
-typedef struct MSIMessage MSIMessage;
+typedef struct NICInfo NICInfo;
 typedef struct NetClientState NetClientState;
 typedef struct NetFilterState NetFilterState;
-typedef struct NICInfo NICInfo;
-typedef struct PcGuestInfo PcGuestInfo;
 typedef struct PCIBridge PCIBridge;
 typedef struct PCIBus PCIBus;
 typedef struct PCIDevice PCIDevice;
@@ -68,31 +64,32 @@ typedef struct PCIExpressDevice PCIExpressDevice;
 typedef struct PCIExpressHost PCIExpressHost;
 typedef struct PCIHostDeviceAddress PCIHostDeviceAddress;
 typedef struct PCIHostState PCIHostState;
+typedef struct PCMCIACardState PCMCIACardState;
 typedef struct PCMachineClass PCMachineClass;
 typedef struct PCMachineState PCMachineState;
-typedef struct PCMCIACardState PCMCIACardState;
+typedef struct PcGuestInfo PcGuestInfo;
 typedef struct PixelFormat PixelFormat;
 typedef struct PostcopyDiscardState PostcopyDiscardState;
 typedef struct Property Property;
 typedef struct PropertyInfo PropertyInfo;
 typedef struct QEMUBH QEMUBH;
-typedef struct QemuConsole QemuConsole;
 typedef struct QEMUFile QEMUFile;
-typedef struct QemuOpt QemuOpt;
-typedef struct QemuOpts QemuOpts;
-typedef struct QemuOptsList QemuOptsList;
 typedef struct QEMUSGList QEMUSGList;
 typedef struct QEMUTimer QEMUTimer;
 typedef struct QEMUTimerListGroup QEMUTimerListGroup;
 typedef struct QObject QObject;
+typedef struct QemuConsole QemuConsole;
+typedef struct QemuOpt QemuOpt;
+typedef struct QemuOpts QemuOpts;
+typedef struct QemuOptsList QemuOptsList;
 typedef struct RAMBlock RAMBlock;
 typedef struct Range Range;
-typedef struct SerialState SerialState;
 typedef struct SHPCDevice SHPCDevice;
 typedef struct SMBusDevice SMBusDevice;
 typedef struct SSIBus SSIBus;
-typedef struct uWireSlave uWireSlave;
+typedef struct SerialState SerialState;
 typedef struct VirtIODevice VirtIODevice;
 typedef struct Visitor Visitor;
+typedef struct uWireSlave uWireSlave;
 
 #endif /* QEMU_TYPEDEFS_H */
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH RFC 4/5] include: Move typedef qemu_irq to qemu/typedefs.h
  2016-06-23 16:12 [Qemu-devel] [PATCH RFC 0/5] Baby steps towards saner headers Markus Armbruster
                   ` (2 preceding siblings ...)
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 3/5] include/qemu/typedefs.h: Restore alphabetical order Markus Armbruster
@ 2016-06-23 16:12 ` Markus Armbruster
  2016-06-23 16:41   ` Peter Maydell
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 5/5] include: Include exec/hwaddr.h where hwaddr is used Markus Armbruster
  4 siblings, 1 reply; 25+ messages in thread
From: Markus Armbruster @ 2016-06-23 16:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, mst, pbonzini

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/hw/irq.h        | 2 --
 include/qemu/typedefs.h | 1 +
 tests/Makefile.include  | 2 --
 3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/hw/irq.h b/include/hw/irq.h
index 4c4c2ea..6a89571 100644
--- a/include/hw/irq.h
+++ b/include/hw/irq.h
@@ -5,8 +5,6 @@
 
 #define TYPE_IRQ "irq"
 
-typedef struct IRQState *qemu_irq;
-
 typedef void (*qemu_irq_handler)(void *opaque, int n, int level);
 
 void qemu_set_irq(qemu_irq irq, int level);
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index f9745c4..f7c27a9 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -31,6 +31,7 @@ typedef struct FWCfgState FWCfgState;
 typedef struct HCIInfo HCIInfo;
 typedef struct I2CBus I2CBus;
 typedef struct I2SCodec I2SCodec;
+typedef struct IRQState *qemu_irq;
 typedef struct ISABus ISABus;
 typedef struct ISADevice ISADevice;
 typedef struct IsaDma IsaDma;
diff --git a/tests/Makefile.include b/tests/Makefile.include
index e20f437..cd2082a 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -543,11 +543,9 @@ blacklisted-headers :=					\
 	include/hw/input/hid.h				\
 	include/hw/intc/allwinner-a10-pic.h		\
 	include/hw/isa/i8257.h				\
-	include/hw/isa/vt82c686.h			\
 	include/hw/kvm/clock.h				\
 	include/hw/mips/bios.h				\
 	include/hw/mips/cpudevs.h			\
-	include/hw/mips/mips.h				\
 	include/hw/misc/mips_cmgcr.h			\
 	include/hw/misc/mips_cpc.h			\
 	include/hw/misc/mips_itu.h			\
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH RFC 5/5] include: Include exec/hwaddr.h where hwaddr is used
  2016-06-23 16:12 [Qemu-devel] [PATCH RFC 0/5] Baby steps towards saner headers Markus Armbruster
                   ` (3 preceding siblings ...)
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 4/5] include: Move typedef qemu_irq to qemu/typedefs.h Markus Armbruster
@ 2016-06-23 16:12 ` Markus Armbruster
  2016-06-24  8:17   ` Markus Armbruster
  4 siblings, 1 reply; 25+ messages in thread
From: Markus Armbruster @ 2016-06-23 16:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, mst, pbonzini

Don't bother with include/exec/memory-internal.h and
include/hw/elf_ops.h, because those are somewhat special.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/audio/lm4549.h                 | 1 +
 hw/cris/boot.h                    | 2 ++
 hw/net/e1000e_core.h              | 7 +++++++
 hw/net/vmware_utils.h             | 1 +
 hw/scsi/mptsas.h                  | 1 +
 include/disas/disas.h             | 1 +
 include/hw/arm/sharpsl.h          | 3 +++
 include/hw/block/fdc.h            | 1 +
 include/hw/char/escc.h            | 2 ++
 include/hw/char/pl011.h           | 2 ++
 include/hw/char/xilinx_uartlite.h | 2 ++
 include/hw/cris/etraxfs_dma.h     | 2 ++
 include/hw/empty_slot.h           | 2 ++
 include/hw/misc/mips_cmgcr.h      | 1 +
 include/hw/pci-host/apb.h         | 1 +
 include/hw/sparc/sparc32_dma.h    | 2 ++
 include/hw/timer/m48t59.h         | 1 +
 include/sysemu/xen-mapcache.h     | 1 +
 target-ppc/mmu-hash32.h           | 2 ++
 tests/Makefile.include            | 9 ---------
 20 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/hw/audio/lm4549.h b/hw/audio/lm4549.h
index 812a7a4..8e6eebf 100644
--- a/hw/audio/lm4549.h
+++ b/hw/audio/lm4549.h
@@ -13,6 +13,7 @@
 #define HW_LM4549_H
 
 #include "audio/audio.h"
+#include "exec/hwaddr.h"
 
 typedef void (*lm4549_callback)(void *opaque);
 
diff --git a/hw/cris/boot.h b/hw/cris/boot.h
index c4d3fa6..a51ae92 100644
--- a/hw/cris/boot.h
+++ b/hw/cris/boot.h
@@ -1,6 +1,8 @@
 #ifndef _CRIS_BOOT_H
 #define HW_CRIS_BOOT_H 1
 
+#include "exec/hwaddr.h"
+
 struct cris_load_info
 {
     const char *image_filename;
diff --git a/hw/net/e1000e_core.h b/hw/net/e1000e_core.h
index 5f413a9..2c903f9 100644
--- a/hw/net/e1000e_core.h
+++ b/hw/net/e1000e_core.h
@@ -33,6 +33,11 @@
 * License along with this library; if not, see <http://www.gnu.org/licenses/>.
 */
 
+#ifndef E1000E_CORE_H
+#define E1000E_CORE_H
+
+#include "exec/hwaddr.h"
+
 #define E1000E_PHY_PAGE_SIZE    (0x20)
 #define E1000E_PHY_PAGES        (0x07)
 #define E1000E_MAC_SIZE         (0x8000)
@@ -144,3 +149,5 @@ e1000e_receive(E1000ECore *core, const uint8_t *buf, size_t size);
 
 ssize_t
 e1000e_receive_iov(E1000ECore *core, const struct iovec *iov, int iovcnt);
+
+#endif
diff --git a/hw/net/vmware_utils.h b/hw/net/vmware_utils.h
index c0dbb2f..9ca714d 100644
--- a/hw/net/vmware_utils.h
+++ b/hw/net/vmware_utils.h
@@ -17,6 +17,7 @@
 #ifndef VMWARE_UTILS_H
 #define VMWARE_UTILS_H
 
+#include "exec/hwaddr.h"
 #include "qemu/range.h"
 #include "vmxnet_debug.h"
 
diff --git a/hw/scsi/mptsas.h b/hw/scsi/mptsas.h
index 595f81f..553552d 100644
--- a/hw/scsi/mptsas.h
+++ b/hw/scsi/mptsas.h
@@ -1,6 +1,7 @@
 #ifndef MPTSAS_H
 #define MPTSAS_H
 
+#include "exec/hwaddr.h"
 #include "mpi.h"
 
 #define MPTSAS_NUM_PORTS 8
diff --git a/include/disas/disas.h b/include/disas/disas.h
index 4930d78..8e221c3 100644
--- a/include/disas/disas.h
+++ b/include/disas/disas.h
@@ -1,6 +1,7 @@
 #ifndef _QEMU_DISAS_H
 #define _QEMU_DISAS_H
 
+#include "exec/hwaddr.h"
 #include "qemu-common.h"
 
 #ifdef NEED_CPU_H
diff --git a/include/hw/arm/sharpsl.h b/include/hw/arm/sharpsl.h
index 13981a6..b55d95a 100644
--- a/include/hw/arm/sharpsl.h
+++ b/include/hw/arm/sharpsl.h
@@ -3,9 +3,12 @@
  *
  * This file is licensed under the GNU GPL.
  */
+
 #ifndef QEMU_SHARPSL_H
 #define QEMU_SHARPSL_H
 
+#include "exec/hwaddr.h"
+
 #define zaurus_printf(format, ...)	\
     fprintf(stderr, "%s: " format, __FUNCTION__, ##__VA_ARGS__)
 
diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
index 1749dab..535ec90 100644
--- a/include/hw/block/fdc.h
+++ b/include/hw/block/fdc.h
@@ -1,6 +1,7 @@
 #ifndef HW_FDC_H
 #define HW_FDC_H
 
+#include "exec/hwaddr.h"
 #include "qemu-common.h"
 
 /* fdc.c */
diff --git a/include/hw/char/escc.h b/include/hw/char/escc.h
index 2742d70..ead9444 100644
--- a/include/hw/char/escc.h
+++ b/include/hw/char/escc.h
@@ -1,6 +1,8 @@
 #ifndef HW_ESCC_H
 #define HW_ESCC_H 1
 
+#include "exec/hwaddr.h"
+
 /* escc.c */
 #define TYPE_ESCC "escc"
 #define ESCC_SIZE 4
diff --git a/include/hw/char/pl011.h b/include/hw/char/pl011.h
index 93bd7ee..e546030 100644
--- a/include/hw/char/pl011.h
+++ b/include/hw/char/pl011.h
@@ -15,6 +15,8 @@
 #ifndef PL011_UART_H
 #define PL011_UART_H
 
+#include "exec/hwaddr.h"
+
 static inline DeviceState *pl011_create(hwaddr addr,
                                         qemu_irq irq,
                                         CharDriverState *chr)
diff --git a/include/hw/char/xilinx_uartlite.h b/include/hw/char/xilinx_uartlite.h
index 8b4fc54..3e8a914 100644
--- a/include/hw/char/xilinx_uartlite.h
+++ b/include/hw/char/xilinx_uartlite.h
@@ -15,6 +15,8 @@
 #ifndef XILINX_UARTLITE_H
 #define XILINX_UARTLITE_H
 
+#include "exec/hwaddr.h"
+
 static inline DeviceState *xilinx_uartlite_create(hwaddr addr,
                                         qemu_irq irq,
                                         CharDriverState *chr)
diff --git a/include/hw/cris/etraxfs_dma.h b/include/hw/cris/etraxfs_dma.h
index 38104a6..ed985b5 100644
--- a/include/hw/cris/etraxfs_dma.h
+++ b/include/hw/cris/etraxfs_dma.h
@@ -1,6 +1,8 @@
 #ifndef HW_ETRAXFS_DMA_H
 #define HW_ETRAXFS_DMA_H 1
 
+#include "exec/hwaddr.h"
+
 struct dma_context_metadata {
 	/* data descriptor md */
 	uint16_t metadata;
diff --git a/include/hw/empty_slot.h b/include/hw/empty_slot.h
index 6079602..64b4b02 100644
--- a/include/hw/empty_slot.h
+++ b/include/hw/empty_slot.h
@@ -1,6 +1,8 @@
 #ifndef HW_EMPTY_SLOT_H
 #define HW_EMPTY_SLOT_H 1
 
+#include "exec/hwaddr.h"
+
 /* empty_slot.c */
 void empty_slot_init(hwaddr addr, uint64_t slot_size);
 
diff --git a/include/hw/misc/mips_cmgcr.h b/include/hw/misc/mips_cmgcr.h
index cc60eef..3c823ea 100644
--- a/include/hw/misc/mips_cmgcr.h
+++ b/include/hw/misc/mips_cmgcr.h
@@ -11,6 +11,7 @@
 #define _MIPS_GCR_H
 
 #define TYPE_MIPS_GCR "mips-gcr"
+#include "exec/hwaddr.h"
 #define MIPS_GCR(obj) OBJECT_CHECK(MIPSGCRState, (obj), TYPE_MIPS_GCR)
 
 #define GCR_BASE_ADDR           0x1fbf8000ULL
diff --git a/include/hw/pci-host/apb.h b/include/hw/pci-host/apb.h
index 736db61..824b5c6 100644
--- a/include/hw/pci-host/apb.h
+++ b/include/hw/pci-host/apb.h
@@ -1,6 +1,7 @@
 #ifndef APB_PCI_H
 #define APB_PCI_H
 
+#include "exec/hwaddr.h"
 #include "qemu-common.h"
 
 PCIBus *pci_apb_init(hwaddr special_base,
diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
index 9497b13..fbc95fd 100644
--- a/include/hw/sparc/sparc32_dma.h
+++ b/include/hw/sparc/sparc32_dma.h
@@ -1,6 +1,8 @@
 #ifndef SPARC32_DMA_H
 #define SPARC32_DMA_H
 
+#include "exec/hwaddr.h"
+
 /* sparc32_dma.c */
 void ledma_memory_read(void *opaque, hwaddr addr,
                        uint8_t *buf, int len, int do_bswap);
diff --git a/include/hw/timer/m48t59.h b/include/hw/timer/m48t59.h
index 3367923..aca90b3 100644
--- a/include/hw/timer/m48t59.h
+++ b/include/hw/timer/m48t59.h
@@ -1,6 +1,7 @@
 #ifndef NVRAM_H
 #define NVRAM_H
 
+#include "exec/hwaddr.h"
 #include "qemu-common.h"
 #include "qom/object.h"
 
diff --git a/include/sysemu/xen-mapcache.h b/include/sysemu/xen-mapcache.h
index c849489..93f3ecf 100644
--- a/include/sysemu/xen-mapcache.h
+++ b/include/sysemu/xen-mapcache.h
@@ -9,6 +9,7 @@
 #ifndef XEN_MAPCACHE_H
 #define XEN_MAPCACHE_H
 
+#include "exec/hwaddr.h"
 
 typedef hwaddr (*phys_offset_to_gaddr_t)(hwaddr start_addr,
                                                      ram_addr_t size,
diff --git a/target-ppc/mmu-hash32.h b/target-ppc/mmu-hash32.h
index aaceacd..c9d04e4 100644
--- a/target-ppc/mmu-hash32.h
+++ b/target-ppc/mmu-hash32.h
@@ -3,6 +3,8 @@
 
 #ifndef CONFIG_USER_ONLY
 
+#include "exec/hwaddr.h"
+
 hwaddr get_pteg_offset32(PowerPCCPU *cpu, hwaddr hash);
 hwaddr ppc_hash32_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
 int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr address, int rw,
diff --git a/tests/Makefile.include b/tests/Makefile.include
index cd2082a..732d6fa 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -496,7 +496,6 @@ blacklisted-headers :=					\
 	hw/usb/quirks.h					\
 	hw/xtensa/bootparam.h				\
 	include/block/write-threshold.h			\
-	include/disas/disas.h				\
 	include/exec/cpu-all.h				\
 	include/exec/cpu-defs.h				\
 	include/exec/cpu_ldst.h				\
@@ -523,17 +522,12 @@ blacklisted-headers :=					\
 	include/hw/arm/fsl-imx25.h			\
 	include/hw/arm/fsl-imx31.h			\
 	include/hw/arm/fsl-imx6.h			\
-	include/hw/arm/sharpsl.h			\
 	include/hw/arm/xlnx-zynqmp.h			\
-	include/hw/block/fdc.h				\
 	include/hw/block/flash.h			\
-	include/hw/char/escc.h				\
 	include/hw/char/pl011.h				\
 	include/hw/char/xilinx_uartlite.h		\
 	include/hw/cris/etraxfs.h			\
-	include/hw/cris/etraxfs_dma.h			\
 	include/hw/elf_ops.h				\
-	include/hw/empty_slot.h				\
 	include/hw/i2c/aspeed_i2c.h			\
 	include/hw/i2c/i2c-ddc.h			\
 	include/hw/i2c/pm_smbus.h			\
@@ -551,7 +545,6 @@ blacklisted-headers :=					\
 	include/hw/misc/mips_itu.h			\
 	include/hw/net/allwinner_emac.h			\
 	include/hw/nvram/openbios_firmware_abi.h	\
-	include/hw/pci-host/apb.h			\
 	include/hw/pci-host/spapr.h			\
 	include/hw/pci/pci_bus.h			\
 	include/hw/pci/pcie_aer.h			\
@@ -562,12 +555,10 @@ blacklisted-headers :=					\
 	include/hw/ppc/spapr_vio.h			\
 	include/hw/ppc/xics.h				\
 	include/hw/sparc/grlib.h			\
-	include/hw/sparc/sparc32_dma.h			\
 	include/hw/ssi/xilinx_spips.h			\
 	include/hw/timer/allwinner-a10-pit.h		\
 	include/hw/timer/aspeed_timer.h			\
 	include/hw/timer/i8254_internal.h		\
-	include/hw/timer/m48t59.h			\
 	include/hw/virtio/virtio-access.h		\
 	include/hw/virtio/virtio-input.h		\
 	include/hw/virtio/virtio-rng.h			\
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 1/5] Use #include "..." exactly for our own headers
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 1/5] Use #include "..." exactly for our own headers Markus Armbruster
@ 2016-06-23 16:31   ` Peter Maydell
  0 siblings, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2016-06-23 16:31 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: QEMU Developers, Paolo Bonzini, Michael S. Tsirkin

On 23 June 2016 at 17:12, Markus Armbruster <armbru@redhat.com> wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block/iscsi.c                   | 1 -
>  crypto/pbkdf-gcrypt.c           | 2 +-
>  crypto/pbkdf-nettle.c           | 2 +-
>  exec.c                          | 2 +-
>  hw/scsi/virtio-scsi-dataplane.c | 2 +-
>  hw/scsi/virtio-scsi.c           | 2 +-
>  linux-user/flatload.c           | 2 +-
>  monitor.c                       | 2 +-
>  slirp/bootp.c                   | 2 +-
>  slirp/cksum.c                   | 2 +-
>  slirp/if.c                      | 2 +-
>  slirp/ip_input.c                | 2 +-
>  slirp/ip_output.c               | 2 +-
>  slirp/mbuf.c                    | 2 +-
>  slirp/misc.c                    | 5 ++---
>  slirp/sbuf.c                    | 4 ++--
>  slirp/socket.c                  | 2 +-
>  slirp/tcp_input.c               | 2 +-
>  slirp/tcp_output.c              | 2 +-
>  slirp/tcp_subr.c                | 2 +-
>  slirp/tcp_timer.c               | 2 +-
>  slirp/tftp.c                    | 2 +-
>  slirp/udp.c                     | 2 +-
>  target-arm/arm-powerctl.c       | 4 ++--
>  target-arm/psci.c               | 8 ++++----
>  target-ppc/translate_init.c     | 2 +-
>  target-s390x/cpu.h              | 2 +-
>  target-unicore32/softmmu.c      | 2 +-
>  tests/postcopy-test.c           | 3 +--
>  tests/vhost-user-bridge.c       | 2 --
>  tests/vhost-user-test.c         | 2 +-
>  util/mmap-alloc.c               | 3 ++-
>  util/oslib-posix.c              | 2 +-
>  33 files changed, 38 insertions(+), 42 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 3/5] include/qemu/typedefs.h: Restore alphabetical order
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 3/5] include/qemu/typedefs.h: Restore alphabetical order Markus Armbruster
@ 2016-06-23 16:40   ` Peter Maydell
  2016-06-24  8:11     ` Markus Armbruster
  0 siblings, 1 reply; 25+ messages in thread
From: Peter Maydell @ 2016-06-23 16:40 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: QEMU Developers, Paolo Bonzini, Michael S. Tsirkin

On 23 June 2016 at 17:12, Markus Armbruster <armbru@redhat.com> wrote:
> While there, drop a comment that has become misleading.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

The list is already in alphabetical order, it's just
case insensitive and you have gone for case sensitive.

$ grep typedef include/qemu/typedefs.h >/tmp/unsorted.txt
$ sort -f /tmp/unsorted.txt >/tmp/sorted.txt
$ diff /tmp/unsorted.txt /tmp/sorted.txt

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/5] tests: New make target check-headers
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 2/5] tests: New make target check-headers Markus Armbruster
@ 2016-06-23 16:40   ` Paolo Bonzini
  2016-06-23 16:47     ` Peter Maydell
  2016-06-23 19:33   ` Eric Blake
  1 sibling, 1 reply; 25+ messages in thread
From: Paolo Bonzini @ 2016-06-23 16:40 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: eblake, mst



On 23/06/2016 18:12, Markus Armbruster wrote:
> For each header "FOO.h", the test checks whether
> 
> 	#include "qemu/osdep.h"
> 	#include "FOO.h"
> 	#include "FOO.h"
> 
> compiles.  A large number of headers don't pass this test, by design
> or by accident.  These are all excluded with a blacklist for now.  Add
> make target check-blacklisted-headers to help with examinating how
> they fail.

Can we replace the blacklist with a fixed-format /* FIXME */ comment?

Paolo

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/Makefile.include | 426 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 426 insertions(+)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index a2ed83b..e20f437 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -413,6 +413,421 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
>  $(test-obj-y): QEMU_INCLUDES += -Itests
>  QEMU_CFLAGS += -I$(SRC_PATH)/tests
>  
> +# All headers
> +headers := $(filter %.h, $(shell cd $(SRC_PATH) && git ls-files) $(GENERATED_HEADERS))
> +# Headers that need more than just #include "qemu/osdep.h" to compile
> +blacklisted-headers :=					\
> +	audio/audio_int.h				\
> +	audio/audio_template.h				\
> +	audio/audio_win_int.h				\
> +	audio/dsound_template.h				\
> +	audio/mixeng_template.h				\
> +	audio/rate_template.h				\
> +	block/qcow2.h					\
> +	block/raw-aio.h					\
> +	block/vhdx.h					\
> +	bsd-user/i386/target_signal.h			\
> +	bsd-user/i386/target_syscall.h			\
> +	bsd-user/qemu.h					\
> +	bsd-user/sparc/target_signal.h			\
> +	bsd-user/sparc/target_syscall.h			\
> +	bsd-user/sparc64/target_signal.h		\
> +	bsd-user/sparc64/target_syscall.h		\
> +	bsd-user/syscall_defs.h				\
> +	bsd-user/x86_64/target_signal.h			\
> +	bsd-user/x86_64/target_syscall.h		\
> +	disas/libvixl/vixl/a64/assembler-a64.h		\
> +	disas/libvixl/vixl/a64/constants-a64.h		\
> +	disas/libvixl/vixl/a64/cpu-a64.h		\
> +	disas/libvixl/vixl/a64/decoder-a64.h		\
> +	disas/libvixl/vixl/a64/disasm-a64.h		\
> +	disas/libvixl/vixl/a64/instructions-a64.h	\
> +	disas/libvixl/vixl/code-buffer.h		\
> +	disas/libvixl/vixl/compiler-intrinsics.h	\
> +	disas/libvixl/vixl/globals.h			\
> +	disas/libvixl/vixl/invalset.h			\
> +	disas/libvixl/vixl/platform.h			\
> +	disas/libvixl/vixl/utils.h			\
> +	fpu/softfloat-macros.h				\
> +	fpu/softfloat-specialize.h			\
> +	hw/9pfs/9p-synth.h				\
> +	hw/9pfs/9p-xattr.h				\
> +	hw/audio/fmopl.h				\
> +	hw/audio/hda-codec-common.h			\
> +	hw/audio/lm4549.h				\
> +	hw/block/nvme.h					\
> +	hw/block/xen_blkif.h				\
> +	hw/cris/boot.h					\
> +	hw/display/cirrus_vga_rop.h			\
> +	hw/display/cirrus_vga_rop2.h			\
> +	hw/display/milkymist-vgafb_template.h		\
> +	hw/display/omap_lcd_template.h			\
> +	hw/display/pl110_template.h			\
> +	hw/display/pxa2xx_template.h			\
> +	hw/display/sm501_template.h			\
> +	hw/display/tc6393xb_template.h			\
> +	hw/display/vga-helpers.h			\
> +	hw/display/vga_int.h				\
> +	hw/i386/intel_iommu_internal.h			\
> +	hw/ide/ahci.h					\
> +	hw/lm32/lm32.h					\
> +	hw/lm32/lm32_hwsetup.h				\
> +	hw/lm32/milkymist-hw.h				\
> +	hw/microblaze/boot.h				\
> +	hw/net/e1000e_core.h				\
> +	hw/net/e1000x_common.h				\
> +	hw/net/ne2000.h					\
> +	hw/net/pcnet.h					\
> +	hw/net/rocker/rocker_desc.h			\
> +	hw/net/rocker/rocker_fp.h			\
> +	hw/net/rocker/rocker_of_dpa.h			\
> +	hw/net/rocker/rocker_tlv.h			\
> +	hw/net/rocker/rocker_world.h			\
> +	hw/net/vmware_utils.h				\
> +	hw/ppc/ppc405.h					\
> +	hw/s390x/ipl.h					\
> +	hw/s390x/s390-pci-inst.h			\
> +	hw/s390x/s390-virtio.h				\
> +	hw/scsi/mptsas.h				\
> +	hw/scsi/viosrp.h				\
> +	hw/tpm/tpm_int.h				\
> +	hw/tpm/tpm_tis.h				\
> +	hw/usb/desc.h					\
> +	hw/usb/quirks.h					\
> +	hw/xtensa/bootparam.h				\
> +	include/block/write-threshold.h			\
> +	include/disas/disas.h				\
> +	include/exec/cpu-all.h				\
> +	include/exec/cpu-defs.h				\
> +	include/exec/cpu_ldst.h				\
> +	include/exec/cpu_ldst_template.h		\
> +	include/exec/cpu_ldst_useronly_template.h	\
> +	include/exec/cputlb.h				\
> +	include/exec/exec-all.h				\
> +	include/exec/gen-icount.h			\
> +	include/exec/helper-gen.h			\
> +	include/exec/helper-proto.h			\
> +	include/exec/helper-tcg.h			\
> +	include/exec/ioport.h				\
> +	include/exec/memory-internal.h			\
> +	include/exec/ram_addr.h				\
> +	include/exec/softmmu-semi.h			\
> +	include/exec/tb-hash.h				\
> +	include/exec/user/abitypes.h			\
> +	include/exec/user/thunk.h			\
> +	include/hw/acpi/piix4.h				\
> +	include/hw/acpi/tco.h				\
> +	include/hw/arm/allwinner-a10.h			\
> +	include/hw/arm/bcm2836.h			\
> +	include/hw/arm/digic.h				\
> +	include/hw/arm/fsl-imx25.h			\
> +	include/hw/arm/fsl-imx31.h			\
> +	include/hw/arm/fsl-imx6.h			\
> +	include/hw/arm/sharpsl.h			\
> +	include/hw/arm/xlnx-zynqmp.h			\
> +	include/hw/block/fdc.h				\
> +	include/hw/block/flash.h			\
> +	include/hw/char/escc.h				\
> +	include/hw/char/pl011.h				\
> +	include/hw/char/xilinx_uartlite.h		\
> +	include/hw/cris/etraxfs.h			\
> +	include/hw/cris/etraxfs_dma.h			\
> +	include/hw/elf_ops.h				\
> +	include/hw/empty_slot.h				\
> +	include/hw/i2c/aspeed_i2c.h			\
> +	include/hw/i2c/i2c-ddc.h			\
> +	include/hw/i2c/pm_smbus.h			\
> +	include/hw/i386/apic_internal.h			\
> +	include/hw/i386/intel_iommu.h			\
> +	include/hw/i386/ioapic_internal.h		\
> +	include/hw/input/hid.h				\
> +	include/hw/intc/allwinner-a10-pic.h		\
> +	include/hw/isa/i8257.h				\
> +	include/hw/isa/vt82c686.h			\
> +	include/hw/kvm/clock.h				\
> +	include/hw/mips/bios.h				\
> +	include/hw/mips/cpudevs.h			\
> +	include/hw/mips/mips.h				\
> +	include/hw/misc/mips_cmgcr.h			\
> +	include/hw/misc/mips_cpc.h			\
> +	include/hw/misc/mips_itu.h			\
> +	include/hw/net/allwinner_emac.h			\
> +	include/hw/nvram/openbios_firmware_abi.h	\
> +	include/hw/pci-host/apb.h			\
> +	include/hw/pci-host/spapr.h			\
> +	include/hw/pci/pci_bus.h			\
> +	include/hw/pci/pcie_aer.h			\
> +	include/hw/ppc/ppc.h				\
> +	include/hw/ppc/ppc4xx.h				\
> +	include/hw/ppc/spapr.h				\
> +	include/hw/ppc/spapr_cpu_core.h			\
> +	include/hw/ppc/spapr_vio.h			\
> +	include/hw/ppc/xics.h				\
> +	include/hw/sparc/grlib.h			\
> +	include/hw/sparc/sparc32_dma.h			\
> +	include/hw/ssi/xilinx_spips.h			\
> +	include/hw/timer/allwinner-a10-pit.h		\
> +	include/hw/timer/aspeed_timer.h			\
> +	include/hw/timer/i8254_internal.h		\
> +	include/hw/timer/m48t59.h			\
> +	include/hw/virtio/virtio-access.h		\
> +	include/hw/virtio/virtio-input.h		\
> +	include/hw/virtio/virtio-rng.h			\
> +	include/libdecnumber/decNumberLocal.h		\
> +	include/migration/cpu.h				\
> +	include/monitor/hmp-target.h			\
> +	include/qemu/ratelimit.h			\
> +	include/qemu/thread-win32.h			\
> +	include/sysemu/balloon.h			\
> +	include/sysemu/cpus.h				\
> +	include/sysemu/dump.h				\
> +	include/sysemu/iothread.h			\
> +	include/sysemu/kvm_int.h			\
> +	include/sysemu/memory_mapping.h			\
> +	include/sysemu/os-win32.h			\
> +	include/sysemu/tpm.h				\
> +	include/sysemu/xen-mapcache.h			\
> +	include/trace-tcg.h				\
> +	include/ui/egl-context.h			\
> +	include/ui/egl-helpers.h			\
> +	include/ui/gtk.h				\
> +	include/ui/input.h				\
> +	include/ui/pixel_ops.h				\
> +	include/ui/sdl2.h				\
> +	include/ui/spice-display.h			\
> +	linux-headers/asm-arm64/kvm.h			\
> +	linux-headers/asm-mips/unistd.h			\
> +	linux-headers/asm-powerpc/kvm_para.h		\
> +	linux-user/aarch64/target_cpu.h			\
> +	linux-user/aarch64/target_signal.h		\
> +	linux-user/aarch64/target_structs.h		\
> +	linux-user/aarch64/termbits.h			\
> +	linux-user/alpha/target_cpu.h			\
> +	linux-user/alpha/target_signal.h		\
> +	linux-user/alpha/target_structs.h		\
> +	linux-user/alpha/target_syscall.h		\
> +	linux-user/alpha/termbits.h			\
> +	linux-user/arm/nwfpe/fpa11.h			\
> +	linux-user/arm/nwfpe/fpopcode.h			\
> +	linux-user/arm/target_cpu.h			\
> +	linux-user/arm/target_signal.h			\
> +	linux-user/arm/target_structs.h			\
> +	linux-user/arm/target_syscall.h			\
> +	linux-user/arm/termbits.h			\
> +	linux-user/cris/target_cpu.h			\
> +	linux-user/cris/target_signal.h			\
> +	linux-user/cris/target_structs.h		\
> +	linux-user/cris/termbits.h			\
> +	linux-user/flat.h				\
> +	linux-user/i386/target_cpu.h			\
> +	linux-user/i386/target_signal.h			\
> +	linux-user/i386/target_structs.h		\
> +	linux-user/i386/target_syscall.h		\
> +	linux-user/i386/termbits.h			\
> +	linux-user/ioctls.h				\
> +	linux-user/m68k/target_cpu.h			\
> +	linux-user/m68k/target_signal.h			\
> +	linux-user/m68k/target_structs.h		\
> +	linux-user/m68k/target_syscall.h		\
> +	linux-user/m68k/termbits.h			\
> +	linux-user/microblaze/target_cpu.h		\
> +	linux-user/microblaze/target_signal.h		\
> +	linux-user/microblaze/target_structs.h		\
> +	linux-user/microblaze/termbits.h		\
> +	linux-user/mips/target_cpu.h			\
> +	linux-user/mips/target_signal.h			\
> +	linux-user/mips/target_structs.h		\
> +	linux-user/mips/target_syscall.h		\
> +	linux-user/mips/termbits.h			\
> +	linux-user/mips64/syscall_nr.h			\
> +	linux-user/mips64/target_cpu.h			\
> +	linux-user/mips64/target_signal.h		\
> +	linux-user/mips64/target_structs.h		\
> +	linux-user/mips64/target_syscall.h		\
> +	linux-user/mips64/termbits.h			\
> +	linux-user/openrisc/target_cpu.h		\
> +	linux-user/openrisc/target_signal.h		\
> +	linux-user/openrisc/target_structs.h		\
> +	linux-user/openrisc/target_syscall.h		\
> +	linux-user/openrisc/termbits.h			\
> +	linux-user/ppc/syscall_nr.h			\
> +	linux-user/ppc/target_cpu.h			\
> +	linux-user/ppc/target_signal.h			\
> +	linux-user/ppc/target_structs.h			\
> +	linux-user/ppc/target_syscall.h			\
> +	linux-user/ppc/termbits.h			\
> +	linux-user/qemu.h				\
> +	linux-user/s390x/target_cpu.h			\
> +	linux-user/s390x/target_signal.h		\
> +	linux-user/s390x/target_structs.h		\
> +	linux-user/s390x/target_syscall.h		\
> +	linux-user/s390x/termbits.h			\
> +	linux-user/sh4/target_cpu.h			\
> +	linux-user/sh4/target_signal.h			\
> +	linux-user/sh4/target_structs.h			\
> +	linux-user/sh4/termbits.h			\
> +	linux-user/socket.h				\
> +	linux-user/sparc/target_cpu.h			\
> +	linux-user/sparc/target_signal.h		\
> +	linux-user/sparc/target_structs.h		\
> +	linux-user/sparc/target_syscall.h		\
> +	linux-user/sparc/termbits.h			\
> +	linux-user/sparc64/target_cpu.h			\
> +	linux-user/sparc64/target_signal.h		\
> +	linux-user/sparc64/target_structs.h		\
> +	linux-user/sparc64/target_syscall.h		\
> +	linux-user/sparc64/termbits.h			\
> +	linux-user/syscall_defs.h			\
> +	linux-user/syscall_types.h			\
> +	linux-user/tilegx/target_cpu.h			\
> +	linux-user/tilegx/target_signal.h		\
> +	linux-user/tilegx/target_structs.h		\
> +	linux-user/tilegx/target_syscall.h		\
> +	linux-user/unicore32/target_cpu.h		\
> +	linux-user/unicore32/target_signal.h		\
> +	linux-user/unicore32/target_structs.h		\
> +	linux-user/unicore32/target_syscall.h		\
> +	linux-user/unicore32/termbits.h			\
> +	linux-user/x86_64/target_cpu.h			\
> +	linux-user/x86_64/target_signal.h		\
> +	linux-user/x86_64/target_structs.h		\
> +	linux-user/x86_64/target_syscall.h		\
> +	linux-user/x86_64/termbits.h			\
> +	pc-bios/optionrom/optionrom.h			\
> +	pc-bios/s390-ccw/bootmap.h			\
> +	pc-bios/s390-ccw/cio.h				\
> +	pc-bios/s390-ccw/iplb.h				\
> +	pc-bios/s390-ccw/s390-ccw.h			\
> +	pc-bios/s390-ccw/scsi.h				\
> +	pc-bios/s390-ccw/virtio-scsi.h			\
> +	pc-bios/s390-ccw/virtio.h			\
> +	qemu-options-wrapper.h				\
> +	qga/guest-agent-core.h				\
> +	qga/service-win32.h				\
> +	qga/vss-win32/requester.h			\
> +	qga/vss-win32/vss-common.h			\
> +	replay/replay-internal.h			\
> +	scripts/cocci-macro-file.h			\
> +	slirp/bootp.h					\
> +	slirp/ip.h					\
> +	slirp/ip6_icmp.h				\
> +	slirp/ip_icmp.h					\
> +	slirp/main.h					\
> +	slirp/mbuf.h					\
> +	slirp/misc.h					\
> +	slirp/sbuf.h					\
> +	slirp/socket.h					\
> +	slirp/tcp.h					\
> +	slirp/tcp_timer.h				\
> +	slirp/tcp_var.h					\
> +	slirp/tcpip.h					\
> +	slirp/tftp.h					\
> +	slirp/udp.h					\
> +	softmmu_template.h				\
> +	target-alpha/cpu.h				\
> +	target-alpha/helper.h				\
> +	target-arm/arm_ldst.h				\
> +	target-arm/cpu.h				\
> +	target-arm/helper-a64.h				\
> +	target-arm/helper.h				\
> +	target-arm/internals.h				\
> +	target-arm/kvm_arm.h				\
> +	target-arm/op_addsub.h				\
> +	target-arm/translate.h				\
> +	target-cris/cpu.h				\
> +	target-cris/helper.h				\
> +	target-cris/mmu.h				\
> +	target-i386/cc_helper_template.h		\
> +	target-i386/cpu-qom.h				\
> +	target-i386/cpu.h				\
> +	target-i386/helper.h				\
> +	target-i386/hyperv.h				\
> +	target-i386/kvm_i386.h				\
> +	target-i386/ops_sse.h				\
> +	target-i386/ops_sse_header.h			\
> +	target-i386/shift_helper_template.h		\
> +	target-lm32/cpu.h				\
> +	target-lm32/helper.h				\
> +	target-m68k/cpu.h				\
> +	target-m68k/helper.h				\
> +	target-microblaze/cpu.h				\
> +	target-microblaze/helper.h			\
> +	target-microblaze/mmu.h				\
> +	target-mips/cpu-qom.h				\
> +	target-mips/cpu.h				\
> +	target-mips/helper.h				\
> +	target-mips/kvm_mips.h				\
> +	target-mips/mips-defs.h				\
> +	target-moxie/cpu.h				\
> +	target-moxie/helper.h				\
> +	target-moxie/machine.h				\
> +	target-moxie/mmu.h				\
> +	target-openrisc/cpu.h				\
> +	target-openrisc/exception.h			\
> +	target-openrisc/helper.h			\
> +	target-ppc/cpu-models.h				\
> +	target-ppc/cpu-qom.h				\
> +	target-ppc/cpu.h				\
> +	target-ppc/helper.h				\
> +	target-ppc/helper_regs.h			\
> +	target-ppc/kvm_ppc.h				\
> +	target-ppc/mmu-hash32.h				\
> +	target-ppc/mmu-hash64.h				\
> +	target-s390x/cpu.h				\
> +	target-s390x/helper.h				\
> +	target-sh4/cpu.h				\
> +	target-sh4/helper.h				\
> +	target-sparc/cpu-qom.h				\
> +	target-sparc/cpu.h				\
> +	target-sparc/helper.h				\
> +	target-tilegx/cpu.h				\
> +	target-tilegx/helper.h				\
> +	target-tricore/cpu.h				\
> +	target-tricore/helper.h				\
> +	target-tricore/tricore-defs.h			\
> +	target-tricore/tricore-opcodes.h		\
> +	target-unicore32/cpu.h				\
> +	target-unicore32/helper.h			\
> +	target-xtensa/cpu.h				\
> +	target-xtensa/helper.h				\
> +	target-xtensa/overlay_tool.h			\
> +	tcg/mips/tcg-target.h				\
> +	tcg/tcg-be-ldst.h				\
> +	tcg/tcg-be-null.h				\
> +	tcg/tcg-op.h					\
> +	tcg/tcg-opc.h					\
> +	tcg/tcg-runtime.h				\
> +	tcg/tcg.h					\
> +	tests/crypto-tls-x509-helpers.h			\
> +	tests/multiboot/libc.h				\
> +	tests/multiboot/multiboot.h			\
> +	tests/tcg/cris/crisutils.h			\
> +	tests/tcg/cris/sys.h				\
> +	tests/tcg/mips/mips64-dsp/io.h			\
> +	tests/tcg/mips/mips64-dspr2/io.h		\
> +	tests/tcg/test-i386-muldiv.h			\
> +	tests/tcg/test-i386-shift.h			\
> +	tests/tcg/test-i386.h				\
> +	trace/control-internal.h			\
> +	trace/generated-tcg-tracers.h			\
> +	trace/mem-internal.h				\
> +	trace/mem.h					\
> +	trace/simple.h					\
> +	translate-all.h					\
> +	ui/curses_keys.h				\
> +	ui/sdl2-keymap.h				\
> +	ui/sdl_keysym.h					\
> +	ui/sdl_zoom.h					\
> +	ui/sdl_zoom_template.h				\
> +	ui/vgafont.h					\
> +	ui/vnc-auth-sasl.h				\
> +	ui/vnc-auth-vencrypt.h				\
> +	ui/vnc-enc-hextile-template.h			\
> +	ui/vnc-jobs.h					\
> +	ui/vnc-ws.h					\
> +	ui/vnc_keysym.h
> +checked-headers := $(filter-out $(blacklisted-headers), $(headers))
> +-include $(patsubst %.h,tests/headers/%.d, $(checked-headers))
>  
>  # Deps that are common to various different sets of tests below
>  test-util-obj-y = libqemuutil.a libqemustub.a
> @@ -647,6 +1062,7 @@ check-help:
>  	@echo " make check-unit           Run qobject tests"
>  	@echo " make check-qapi-schema    Run QAPI schema tests"
>  	@echo " make check-block          Run block tests"
> +	@echo " make check-headers        Run header sanity tests"
>  	@echo " make check-report.html    Generates an HTML test report"
>  	@echo " make check-clean          Clean the tests"
>  	@echo
> @@ -728,8 +1144,18 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json
>  	@perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -q $(SRC_PATH)/$*.err -
>  	@diff -q $(SRC_PATH)/$*.exit $*.test.exit
>  
> +# Header sanity checking
> +
> +tests/headers/%.c: tests/header-test-template.c
> +	@mkdir -p $(dir $@)
> +	@sed 's,@header@,$(subst tests/headers/,,$(@:.c=.h)),' <$< >$@
> +
>  # Consolidated targets
>  
> +.PHONY: check-headers check-blacklisted-headers
> +check-headers: $(patsubst %.h,tests/headers/%.o, $(checked-headers))
> +check-blacklisted-headers: $(patsubst %.h,tests/headers/%.o, $(blacklisted-headers))
> +
>  .PHONY: check-qapi-schema check-qtest check-unit check check-clean
>  check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y))
>  check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 4/5] include: Move typedef qemu_irq to qemu/typedefs.h
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 4/5] include: Move typedef qemu_irq to qemu/typedefs.h Markus Armbruster
@ 2016-06-23 16:41   ` Peter Maydell
  2016-06-24  8:12     ` Markus Armbruster
  0 siblings, 1 reply; 25+ messages in thread
From: Peter Maydell @ 2016-06-23 16:41 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: QEMU Developers, Paolo Bonzini, Michael S. Tsirkin

On 23 June 2016 at 17:12, Markus Armbruster <armbru@redhat.com> wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/hw/irq.h        | 2 --
>  include/qemu/typedefs.h | 1 +
>  tests/Makefile.include  | 2 --
>  3 files changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/include/hw/irq.h b/include/hw/irq.h
> index 4c4c2ea..6a89571 100644
> --- a/include/hw/irq.h
> +++ b/include/hw/irq.h
> @@ -5,8 +5,6 @@
>
>  #define TYPE_IRQ "irq"
>
> -typedef struct IRQState *qemu_irq;
> -
>  typedef void (*qemu_irq_handler)(void *opaque, int n, int level);
>
>  void qemu_set_irq(qemu_irq irq, int level);
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index f9745c4..f7c27a9 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -31,6 +31,7 @@ typedef struct FWCfgState FWCfgState;
>  typedef struct HCIInfo HCIInfo;
>  typedef struct I2CBus I2CBus;
>  typedef struct I2SCodec I2SCodec;
> +typedef struct IRQState *qemu_irq;
>  typedef struct ISABus ISABus;
>  typedef struct ISADevice ISADevice;
>  typedef struct IsaDma IsaDma;

Everything else in typedefs.h is a "typedef struct Thing Thing",
but qemu_irq is different...

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/5] tests: New make target check-headers
  2016-06-23 16:40   ` Paolo Bonzini
@ 2016-06-23 16:47     ` Peter Maydell
  2016-06-24  8:10       ` Markus Armbruster
  0 siblings, 1 reply; 25+ messages in thread
From: Peter Maydell @ 2016-06-23 16:47 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Markus Armbruster, QEMU Developers, Michael S. Tsirkin

On 23 June 2016 at 17:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 23/06/2016 18:12, Markus Armbruster wrote:
>> For each header "FOO.h", the test checks whether
>>
>>       #include "qemu/osdep.h"
>>       #include "FOO.h"
>>       #include "FOO.h"
>>
>> compiles.  A large number of headers don't pass this test, by design
>> or by accident.  These are all excluded with a blacklist for now.  Add
>> make target check-blacklisted-headers to help with examinating how
>> they fail.
>
> Can we replace the blacklist with a fixed-format /* FIXME */ comment?

Ooh. I was going to suggest moving the blacklist into its own
file, but a magic comment is a better idea, less chance of
merge conflicts.

Rather than listing "check-headers" specifically in the help,
I suggest we have a more generic "check-source" for running
code style/etc checks on the sources (of which check-headers
is the only one at first). Then we have a good place to put
new style type checks in future without requiring everybody
to change the command lines they're using to invoke them.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/5] tests: New make target check-headers
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 2/5] tests: New make target check-headers Markus Armbruster
  2016-06-23 16:40   ` Paolo Bonzini
@ 2016-06-23 19:33   ` Eric Blake
  2016-06-24  8:11     ` Markus Armbruster
  1 sibling, 1 reply; 25+ messages in thread
From: Eric Blake @ 2016-06-23 19:33 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: mst, pbonzini

[-- Attachment #1: Type: text/plain, Size: 1273 bytes --]

On 06/23/2016 10:12 AM, Markus Armbruster wrote:
> For each header "FOO.h", the test checks whether
> 
> 	#include "qemu/osdep.h"
> 	#include "FOO.h"
> 	#include "FOO.h"
> 
> compiles.  A large number of headers don't pass this test, by design
> or by accident.  These are all excluded with a blacklist for now.  Add
> make target check-blacklisted-headers to help with examinating how

s/examinating/examining/

> they fail.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/Makefile.include | 426 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 426 insertions(+)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index a2ed83b..e20f437 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -413,6 +413,421 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
>  $(test-obj-y): QEMU_INCLUDES += -Itests
>  QEMU_CFLAGS += -I$(SRC_PATH)/tests
>  
> +# All headers
> +headers := $(filter %.h, $(shell cd $(SRC_PATH) && git ls-files) $(GENERATED_HEADERS))

Hard-coded to only work on a git checkout, but I guess that's okay.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/5] tests: New make target check-headers
  2016-06-23 16:47     ` Peter Maydell
@ 2016-06-24  8:10       ` Markus Armbruster
  0 siblings, 0 replies; 25+ messages in thread
From: Markus Armbruster @ 2016-06-24  8:10 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Michael S. Tsirkin, QEMU Developers

Peter Maydell <peter.maydell@linaro.org> writes:

> On 23 June 2016 at 17:40, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 23/06/2016 18:12, Markus Armbruster wrote:
>>> For each header "FOO.h", the test checks whether
>>>
>>>       #include "qemu/osdep.h"
>>>       #include "FOO.h"
>>>       #include "FOO.h"
>>>
>>> compiles.  A large number of headers don't pass this test, by design
>>> or by accident.  These are all excluded with a blacklist for now.  Add
>>> make target check-blacklisted-headers to help with examinating how
>>> they fail.
>>
>> Can we replace the blacklist with a fixed-format /* FIXME */ comment?

Great idea, will do!

> Ooh. I was going to suggest moving the blacklist into its own
> file, but a magic comment is a better idea, less chance of
> merge conflicts.
>
> Rather than listing "check-headers" specifically in the help,
> I suggest we have a more generic "check-source" for running
> code style/etc checks on the sources (of which check-headers
> is the only one at first). Then we have a good place to put
> new style type checks in future without requiring everybody
> to change the command lines they're using to invoke them.

Sure.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 2/5] tests: New make target check-headers
  2016-06-23 19:33   ` Eric Blake
@ 2016-06-24  8:11     ` Markus Armbruster
  0 siblings, 0 replies; 25+ messages in thread
From: Markus Armbruster @ 2016-06-24  8:11 UTC (permalink / raw)
  To: Eric Blake; +Cc: qemu-devel, pbonzini, mst

Eric Blake <eblake@redhat.com> writes:

> On 06/23/2016 10:12 AM, Markus Armbruster wrote:
>> For each header "FOO.h", the test checks whether
>> 
>> 	#include "qemu/osdep.h"
>> 	#include "FOO.h"
>> 	#include "FOO.h"
>> 
>> compiles.  A large number of headers don't pass this test, by design
>> or by accident.  These are all excluded with a blacklist for now.  Add
>> make target check-blacklisted-headers to help with examinating how
>
> s/examinating/examining/
>
>> they fail.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  tests/Makefile.include | 426 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 426 insertions(+)
>> 
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index a2ed83b..e20f437 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -413,6 +413,421 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
>>  $(test-obj-y): QEMU_INCLUDES += -Itests
>>  QEMU_CFLAGS += -I$(SRC_PATH)/tests
>>  
>> +# All headers
>> +headers := $(filter %.h, $(shell cd $(SRC_PATH) && git ls-files) $(GENERATED_HEADERS))
>
> Hard-coded to only work on a git checkout, but I guess that's okay.

I'll mention it in the commit message.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 3/5] include/qemu/typedefs.h: Restore alphabetical order
  2016-06-23 16:40   ` Peter Maydell
@ 2016-06-24  8:11     ` Markus Armbruster
  0 siblings, 0 replies; 25+ messages in thread
From: Markus Armbruster @ 2016-06-24  8:11 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, QEMU Developers, Michael S. Tsirkin

Peter Maydell <peter.maydell@linaro.org> writes:

> On 23 June 2016 at 17:12, Markus Armbruster <armbru@redhat.com> wrote:
>> While there, drop a comment that has become misleading.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
> The list is already in alphabetical order, it's just
> case insensitive and you have gone for case sensitive.
>
> $ grep typedef include/qemu/typedefs.h >/tmp/unsorted.txt
> $ sort -f /tmp/unsorted.txt >/tmp/sorted.txt
> $ diff /tmp/unsorted.txt /tmp/sorted.txt

Sorry for the noise then...

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 4/5] include: Move typedef qemu_irq to qemu/typedefs.h
  2016-06-23 16:41   ` Peter Maydell
@ 2016-06-24  8:12     ` Markus Armbruster
  2016-06-24  8:15       ` Peter Maydell
  0 siblings, 1 reply; 25+ messages in thread
From: Markus Armbruster @ 2016-06-24  8:12 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, QEMU Developers, Michael S. Tsirkin

Peter Maydell <peter.maydell@linaro.org> writes:

> On 23 June 2016 at 17:12, Markus Armbruster <armbru@redhat.com> wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  include/hw/irq.h        | 2 --
>>  include/qemu/typedefs.h | 1 +
>>  tests/Makefile.include  | 2 --
>>  3 files changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/include/hw/irq.h b/include/hw/irq.h
>> index 4c4c2ea..6a89571 100644
>> --- a/include/hw/irq.h
>> +++ b/include/hw/irq.h
>> @@ -5,8 +5,6 @@
>>
>>  #define TYPE_IRQ "irq"
>>
>> -typedef struct IRQState *qemu_irq;
>> -
>>  typedef void (*qemu_irq_handler)(void *opaque, int n, int level);
>>
>>  void qemu_set_irq(qemu_irq irq, int level);
>> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
>> index f9745c4..f7c27a9 100644
>> --- a/include/qemu/typedefs.h
>> +++ b/include/qemu/typedefs.h
>> @@ -31,6 +31,7 @@ typedef struct FWCfgState FWCfgState;
>>  typedef struct HCIInfo HCIInfo;
>>  typedef struct I2CBus I2CBus;
>>  typedef struct I2SCodec I2SCodec;
>> +typedef struct IRQState *qemu_irq;
>>  typedef struct ISABus ISABus;
>>  typedef struct ISADevice ISADevice;
>>  typedef struct IsaDma IsaDma;
>
> Everything else in typedefs.h is a "typedef struct Thing Thing",
> but qemu_irq is different...

We want to keep our readers on their toes!

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 4/5] include: Move typedef qemu_irq to qemu/typedefs.h
  2016-06-24  8:12     ` Markus Armbruster
@ 2016-06-24  8:15       ` Peter Maydell
  2016-06-24  8:17         ` Paolo Bonzini
  0 siblings, 1 reply; 25+ messages in thread
From: Peter Maydell @ 2016-06-24  8:15 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Paolo Bonzini, QEMU Developers, Michael S. Tsirkin

On 24 June 2016 at 09:12, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On 23 June 2016 at 17:12, Markus Armbruster <armbru@redhat.com> wrote:
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>  include/hw/irq.h        | 2 --
>>>  include/qemu/typedefs.h | 1 +
>>>  tests/Makefile.include  | 2 --
>>>  3 files changed, 1 insertion(+), 4 deletions(-)
>>>
>>> diff --git a/include/hw/irq.h b/include/hw/irq.h
>>> index 4c4c2ea..6a89571 100644
>>> --- a/include/hw/irq.h
>>> +++ b/include/hw/irq.h
>>> @@ -5,8 +5,6 @@
>>>
>>>  #define TYPE_IRQ "irq"
>>>
>>> -typedef struct IRQState *qemu_irq;
>>> -
>>>  typedef void (*qemu_irq_handler)(void *opaque, int n, int level);
>>>
>>>  void qemu_set_irq(qemu_irq irq, int level);
>>> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
>>> index f9745c4..f7c27a9 100644
>>> --- a/include/qemu/typedefs.h
>>> +++ b/include/qemu/typedefs.h
>>> @@ -31,6 +31,7 @@ typedef struct FWCfgState FWCfgState;
>>>  typedef struct HCIInfo HCIInfo;
>>>  typedef struct I2CBus I2CBus;
>>>  typedef struct I2SCodec I2SCodec;
>>> +typedef struct IRQState *qemu_irq;
>>>  typedef struct ISABus ISABus;
>>>  typedef struct ISADevice ISADevice;
>>>  typedef struct IsaDma IsaDma;
>>
>> Everything else in typedefs.h is a "typedef struct Thing Thing",
>> but qemu_irq is different...
>
> We want to keep our readers on their toes!

It would mean you now have to decide whether the file is orderd
by the types being defined or by the underlying implementation
type (previously both orders were the same)...

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 5/5] include: Include exec/hwaddr.h where hwaddr is used
  2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 5/5] include: Include exec/hwaddr.h where hwaddr is used Markus Armbruster
@ 2016-06-24  8:17   ` Markus Armbruster
  2016-06-24  8:18     ` Paolo Bonzini
  0 siblings, 1 reply; 25+ messages in thread
From: Markus Armbruster @ 2016-06-24  8:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: eblake, mst, pbonzini

Markus Armbruster <armbru@redhat.com> writes:

> Don't bother with include/exec/memory-internal.h and
> include/hw/elf_ops.h, because those are somewhat special.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/audio/lm4549.h                 | 1 +
>  hw/cris/boot.h                    | 2 ++
>  hw/net/e1000e_core.h              | 7 +++++++
>  hw/net/vmware_utils.h             | 1 +
>  hw/scsi/mptsas.h                  | 1 +
>  include/disas/disas.h             | 1 +
>  include/hw/arm/sharpsl.h          | 3 +++
>  include/hw/block/fdc.h            | 1 +
>  include/hw/char/escc.h            | 2 ++
>  include/hw/char/pl011.h           | 2 ++
>  include/hw/char/xilinx_uartlite.h | 2 ++
>  include/hw/cris/etraxfs_dma.h     | 2 ++
>  include/hw/empty_slot.h           | 2 ++
>  include/hw/misc/mips_cmgcr.h      | 1 +
>  include/hw/pci-host/apb.h         | 1 +
>  include/hw/sparc/sparc32_dma.h    | 2 ++
>  include/hw/timer/m48t59.h         | 1 +
>  include/sysemu/xen-mapcache.h     | 1 +
>  target-ppc/mmu-hash32.h           | 2 ++
>  tests/Makefile.include            | 9 ---------
>  20 files changed, 35 insertions(+), 9 deletions(-)

I think the churn is tolerable, but I should mention possible
alternatives:

* Move the typedef to osdep.h.  I don't like it, because it spreads
  hwaddr's 64-bitness over two places.

* Include hwaddr.h from osdep.h

If you have a preference, let me know.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 4/5] include: Move typedef qemu_irq to qemu/typedefs.h
  2016-06-24  8:15       ` Peter Maydell
@ 2016-06-24  8:17         ` Paolo Bonzini
  2016-06-24  9:46           ` Peter Maydell
  0 siblings, 1 reply; 25+ messages in thread
From: Paolo Bonzini @ 2016-06-24  8:17 UTC (permalink / raw)
  To: Peter Maydell, Markus Armbruster; +Cc: QEMU Developers, Michael S. Tsirkin



On 24/06/2016 10:15, Peter Maydell wrote:
>>>> >>> @@ -31,6 +31,7 @@ typedef struct FWCfgState FWCfgState;
>>>> >>>  typedef struct HCIInfo HCIInfo;
>>>> >>>  typedef struct I2CBus I2CBus;
>>>> >>>  typedef struct I2SCodec I2SCodec;
>>>> >>> +typedef struct IRQState *qemu_irq;
>>>> >>>  typedef struct ISABus ISABus;
>>>> >>>  typedef struct ISADevice ISADevice;
>>>> >>>  typedef struct IsaDma IsaDma;
>>> >>
>>> >> Everything else in typedefs.h is a "typedef struct Thing Thing",
>>> >> but qemu_irq is different...
>> >
>> > We want to keep our readers on their toes!
> It would mean you now have to decide whether the file is orderd
> by the types being defined or by the underlying implementation
> type (previously both orders were the same)...

Indeed, and renaming the struct is trivial because it's used in a
handful of places only.

Paolo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 5/5] include: Include exec/hwaddr.h where hwaddr is used
  2016-06-24  8:17   ` Markus Armbruster
@ 2016-06-24  8:18     ` Paolo Bonzini
  0 siblings, 0 replies; 25+ messages in thread
From: Paolo Bonzini @ 2016-06-24  8:18 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: eblake, mst



On 24/06/2016 10:17, Markus Armbruster wrote:
>> > Don't bother with include/exec/memory-internal.h and
>> > include/hw/elf_ops.h, because those are somewhat special.
>> >
>> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> > ---
>> >  hw/audio/lm4549.h                 | 1 +
>> >  hw/cris/boot.h                    | 2 ++
>> >  hw/net/e1000e_core.h              | 7 +++++++
>> >  hw/net/vmware_utils.h             | 1 +
>> >  hw/scsi/mptsas.h                  | 1 +
>> >  include/disas/disas.h             | 1 +
>> >  include/hw/arm/sharpsl.h          | 3 +++
>> >  include/hw/block/fdc.h            | 1 +
>> >  include/hw/char/escc.h            | 2 ++
>> >  include/hw/char/pl011.h           | 2 ++
>> >  include/hw/char/xilinx_uartlite.h | 2 ++
>> >  include/hw/cris/etraxfs_dma.h     | 2 ++
>> >  include/hw/empty_slot.h           | 2 ++
>> >  include/hw/misc/mips_cmgcr.h      | 1 +
>> >  include/hw/pci-host/apb.h         | 1 +
>> >  include/hw/sparc/sparc32_dma.h    | 2 ++
>> >  include/hw/timer/m48t59.h         | 1 +
>> >  include/sysemu/xen-mapcache.h     | 1 +
>> >  target-ppc/mmu-hash32.h           | 2 ++
>> >  tests/Makefile.include            | 9 ---------
>> >  20 files changed, 35 insertions(+), 9 deletions(-)
> I think the churn is tolerable, but I should mention possible
> alternatives:
> 
> * Move the typedef to osdep.h.  I don't like it, because it spreads
>   hwaddr's 64-bitness over two places.
> 
> * Include hwaddr.h from osdep.h
> 
> If you have a preference, let me know.

I think it's fine as is.

Paolo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 4/5] include: Move typedef qemu_irq to qemu/typedefs.h
  2016-06-24  8:17         ` Paolo Bonzini
@ 2016-06-24  9:46           ` Peter Maydell
  2016-06-24 12:32             ` Markus Armbruster
  0 siblings, 1 reply; 25+ messages in thread
From: Peter Maydell @ 2016-06-24  9:46 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Markus Armbruster, QEMU Developers, Michael S. Tsirkin

On 24 June 2016 at 09:17, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 24/06/2016 10:15, Peter Maydell wrote:
>>>>> >>> @@ -31,6 +31,7 @@ typedef struct FWCfgState FWCfgState;
>>>>> >>>  typedef struct HCIInfo HCIInfo;
>>>>> >>>  typedef struct I2CBus I2CBus;
>>>>> >>>  typedef struct I2SCodec I2SCodec;
>>>>> >>> +typedef struct IRQState *qemu_irq;
>>>>> >>>  typedef struct ISABus ISABus;
>>>>> >>>  typedef struct ISADevice ISADevice;
>>>>> >>>  typedef struct IsaDma IsaDma;
>>>> >>
>>>> >> Everything else in typedefs.h is a "typedef struct Thing Thing",
>>>> >> but qemu_irq is different...
>>> >
>>> > We want to keep our readers on their toes!
>> It would mean you now have to decide whether the file is orderd
>> by the types being defined or by the underlying implementation
>> type (previously both orders were the same)...
>
> Indeed, and renaming the struct is trivial because it's used in a
> handful of places only.

It would still be different by being a pointer-to-Foo, not a Foo.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 4/5] include: Move typedef qemu_irq to qemu/typedefs.h
  2016-06-24  9:46           ` Peter Maydell
@ 2016-06-24 12:32             ` Markus Armbruster
  2016-06-24 12:45               ` Peter Maydell
  0 siblings, 1 reply; 25+ messages in thread
From: Markus Armbruster @ 2016-06-24 12:32 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Michael S. Tsirkin, QEMU Developers

Peter Maydell <peter.maydell@linaro.org> writes:

> On 24 June 2016 at 09:17, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> On 24/06/2016 10:15, Peter Maydell wrote:
>>>>>> >>> @@ -31,6 +31,7 @@ typedef struct FWCfgState FWCfgState;
>>>>>> >>>  typedef struct HCIInfo HCIInfo;
>>>>>> >>>  typedef struct I2CBus I2CBus;
>>>>>> >>>  typedef struct I2SCodec I2SCodec;
>>>>>> >>> +typedef struct IRQState *qemu_irq;
>>>>>> >>>  typedef struct ISABus ISABus;
>>>>>> >>>  typedef struct ISADevice ISADevice;
>>>>>> >>>  typedef struct IsaDma IsaDma;
>>>>> >>
>>>>> >> Everything else in typedefs.h is a "typedef struct Thing Thing",
>>>>> >> but qemu_irq is different...
>>>> >
>>>> > We want to keep our readers on their toes!
>>> It would mean you now have to decide whether the file is orderd
>>> by the types being defined or by the underlying implementation
>>> type (previously both orders were the same)...
>>
>> Indeed, and renaming the struct is trivial because it's used in a
>> handful of places only.
>
> It would still be different by being a pointer-to-Foo, not a Foo.

Hiding pointer-ness behind a typedef is a bad idea more often than not.

What do you want me to do, if anything?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 4/5] include: Move typedef qemu_irq to qemu/typedefs.h
  2016-06-24 12:32             ` Markus Armbruster
@ 2016-06-24 12:45               ` Peter Maydell
  2016-06-24 13:43                 ` Markus Armbruster
  0 siblings, 1 reply; 25+ messages in thread
From: Peter Maydell @ 2016-06-24 12:45 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Paolo Bonzini, Michael S. Tsirkin, QEMU Developers

On 24 June 2016 at 13:32, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
>> It would still be different by being a pointer-to-Foo, not a Foo.
>
> Hiding pointer-ness behind a typedef is a bad idea more often than not.
>
> What do you want me to do, if anything?

I think my underlying issue is that the purpose of typedefs.h
(as I see it) is to define some typedefs for handing around
pointers to opaque objects, but we don't pass around pointers
to qemu_irqs, we pass around actual qemu_irqs. So I don't
really feel like it belongs in this header.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 4/5] include: Move typedef qemu_irq to qemu/typedefs.h
  2016-06-24 12:45               ` Peter Maydell
@ 2016-06-24 13:43                 ` Markus Armbruster
  2016-06-24 13:45                   ` Peter Maydell
  0 siblings, 1 reply; 25+ messages in thread
From: Markus Armbruster @ 2016-06-24 13:43 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, QEMU Developers, Michael S. Tsirkin

Peter Maydell <peter.maydell@linaro.org> writes:

> On 24 June 2016 at 13:32, Markus Armbruster <armbru@redhat.com> wrote:
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>> It would still be different by being a pointer-to-Foo, not a Foo.
>>
>> Hiding pointer-ness behind a typedef is a bad idea more often than not.
>>
>> What do you want me to do, if anything?
>
> I think my underlying issue is that the purpose of typedefs.h
> (as I see it) is to define some typedefs for handing around
> pointers to opaque objects, but we don't pass around pointers
> to qemu_irqs, we pass around actual qemu_irqs. So I don't
> really feel like it belongs in this header.

My view of typedefs.h is more pragmatic: it's for breaking inclusion
cycles at an arc where we get a complete type from FOO.h, but only need
an incomplete one: declare the incomplete type in typedefs.h, complete
it in FOO.h, drop inclusions of FOO.h that don't need the complete type.

Can also be done by creating a FOO-abstract.h that declares the
incomplete type and whatever else makes sense.  Overkill when "whatever
else" is empty, which it often is.

Dropping inclusions that are just for the incomplete can also speed up
compilation.

If you really don't want qemu_irq in typedefs.h, where should it go?

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH RFC 4/5] include: Move typedef qemu_irq to qemu/typedefs.h
  2016-06-24 13:43                 ` Markus Armbruster
@ 2016-06-24 13:45                   ` Peter Maydell
  0 siblings, 0 replies; 25+ messages in thread
From: Peter Maydell @ 2016-06-24 13:45 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: Paolo Bonzini, QEMU Developers, Michael S. Tsirkin

On 24 June 2016 at 14:43, Markus Armbruster <armbru@redhat.com> wrote:
> If you really don't want qemu_irq in typedefs.h, where should it go?

Last time I had a typedef like that I wrote a header for it:
include/qemu/fprintf-fn.h...

thanks
-- PMM

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2016-06-24 13:45 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-23 16:12 [Qemu-devel] [PATCH RFC 0/5] Baby steps towards saner headers Markus Armbruster
2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 1/5] Use #include "..." exactly for our own headers Markus Armbruster
2016-06-23 16:31   ` Peter Maydell
2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 2/5] tests: New make target check-headers Markus Armbruster
2016-06-23 16:40   ` Paolo Bonzini
2016-06-23 16:47     ` Peter Maydell
2016-06-24  8:10       ` Markus Armbruster
2016-06-23 19:33   ` Eric Blake
2016-06-24  8:11     ` Markus Armbruster
2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 3/5] include/qemu/typedefs.h: Restore alphabetical order Markus Armbruster
2016-06-23 16:40   ` Peter Maydell
2016-06-24  8:11     ` Markus Armbruster
2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 4/5] include: Move typedef qemu_irq to qemu/typedefs.h Markus Armbruster
2016-06-23 16:41   ` Peter Maydell
2016-06-24  8:12     ` Markus Armbruster
2016-06-24  8:15       ` Peter Maydell
2016-06-24  8:17         ` Paolo Bonzini
2016-06-24  9:46           ` Peter Maydell
2016-06-24 12:32             ` Markus Armbruster
2016-06-24 12:45               ` Peter Maydell
2016-06-24 13:43                 ` Markus Armbruster
2016-06-24 13:45                   ` Peter Maydell
2016-06-23 16:12 ` [Qemu-devel] [PATCH RFC 5/5] include: Include exec/hwaddr.h where hwaddr is used Markus Armbruster
2016-06-24  8:17   ` Markus Armbruster
2016-06-24  8:18     ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).