All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives
@ 2018-03-21 15:22 Michael S. Tsirkin
  2018-03-21 15:22 ` [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote Michael S. Tsirkin
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2018-03-21 15:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: Yuval Shaia, Marcel Apfelbaum

Our rule right now is to use <> for external headers only.
RDMA code violates that, fix it up.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/rdma/rdma_backend.h        |  2 +-
 hw/rdma/rdma_backend_defs.h   |  2 +-
 hw/rdma/rdma_rm.h             |  2 +-
 hw/rdma/rdma_utils.h          |  6 +++---
 hw/rdma/vmw/pvrdma.h          |  8 ++++----
 hw/rdma/vmw/pvrdma_dev_ring.h |  2 +-
 hw/rdma/rdma_backend.c        |  6 +++---
 hw/rdma/rdma_rm.c             |  6 +++---
 hw/rdma/vmw/pvrdma_cmd.c      |  8 ++++----
 hw/rdma/vmw/pvrdma_dev_ring.c |  8 ++++----
 hw/rdma/vmw/pvrdma_main.c     | 24 ++++++++++++------------
 hw/rdma/vmw/pvrdma_qp_ops.c   |  4 ++--
 12 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/hw/rdma/rdma_backend.h b/hw/rdma/rdma_backend.h
index 68f2b05..abae262 100644
--- a/hw/rdma/rdma_backend.h
+++ b/hw/rdma/rdma_backend.h
@@ -16,7 +16,7 @@
 #ifndef RDMA_BACKEND_H
 #define RDMA_BACKEND_H
 
-#include <qapi/error.h>
+#include "qapi/error.h"
 #include "rdma_rm_defs.h"
 #include "rdma_backend_defs.h"
 
diff --git a/hw/rdma/rdma_backend_defs.h b/hw/rdma/rdma_backend_defs.h
index 837e324..ff5cfc2 100644
--- a/hw/rdma/rdma_backend_defs.h
+++ b/hw/rdma/rdma_backend_defs.h
@@ -17,7 +17,7 @@
 #define RDMA_BACKEND_DEFS_H
 
 #include <infiniband/verbs.h>
-#include <qemu/thread.h>
+#include "qemu/thread.h"
 
 typedef struct RdmaDeviceResources RdmaDeviceResources;
 
diff --git a/hw/rdma/rdma_rm.h b/hw/rdma/rdma_rm.h
index be95c1b..82c1ddd 100644
--- a/hw/rdma/rdma_rm.h
+++ b/hw/rdma/rdma_rm.h
@@ -16,7 +16,7 @@
 #ifndef RDMA_RM_H
 #define RDMA_RM_H
 
-#include <qapi/error.h>
+#include "qapi/error.h"
 #include "rdma_backend_defs.h"
 #include "rdma_rm_defs.h"
 
diff --git a/hw/rdma/rdma_utils.h b/hw/rdma/rdma_utils.h
index cdac910..3dc0789 100644
--- a/hw/rdma/rdma_utils.h
+++ b/hw/rdma/rdma_utils.h
@@ -17,9 +17,9 @@
 #ifndef RDMA_UTILS_H
 #define RDMA_UTILS_H
 
-#include <qemu/osdep.h>
-#include <include/hw/pci/pci.h>
-#include <include/sysemu/dma.h>
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "sysemu/dma.h"
 
 #define pr_info(fmt, ...) \
     fprintf(stdout, "%s: %-20s (%3d): " fmt, "pvrdma",  __func__, __LINE__,\
diff --git a/hw/rdma/vmw/pvrdma.h b/hw/rdma/vmw/pvrdma.h
index b05f94a..8c173cb 100644
--- a/hw/rdma/vmw/pvrdma.h
+++ b/hw/rdma/vmw/pvrdma.h
@@ -16,14 +16,14 @@
 #ifndef PVRDMA_PVRDMA_H
 #define PVRDMA_PVRDMA_H
 
-#include <hw/pci/pci.h>
-#include <hw/pci/msix.h>
+#include "hw/pci/pci.h"
+#include "hw/pci/msix.h"
 
 #include "../rdma_backend_defs.h"
 #include "../rdma_rm_defs.h"
 
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h>
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h>
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h"
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h"
 #include "pvrdma_dev_ring.h"
 
 /* BARs */
diff --git a/hw/rdma/vmw/pvrdma_dev_ring.h b/hw/rdma/vmw/pvrdma_dev_ring.h
index 02a590b..2d0461f 100644
--- a/hw/rdma/vmw/pvrdma_dev_ring.h
+++ b/hw/rdma/vmw/pvrdma_dev_ring.h
@@ -16,7 +16,7 @@
 #ifndef PVRDMA_DEV_RING_H
 #define PVRDMA_DEV_RING_H
 
-#include <qemu/typedefs.h>
+#include "qemu/typedefs.h"
 
 #define MAX_RING_NAME_SZ 32
 
diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index e306fba..f85c7b2 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -13,9 +13,9 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qemu/error-report.h>
-#include <qapi/error.h>
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
 
 #include <infiniband/verbs.h>
 
diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
index b5fc45d..80ffc94 100644
--- a/hw/rdma/rdma_rm.c
+++ b/hw/rdma/rdma_rm.c
@@ -13,9 +13,9 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qapi/error.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "cpu.h"
 
 #include "rdma_utils.h"
 #include "rdma_backend.h"
diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
index 293dfed..a9fa1ef 100644
--- a/hw/rdma/vmw/pvrdma_cmd.c
+++ b/hw/rdma/vmw/pvrdma_cmd.c
@@ -13,9 +13,9 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qemu/error-report.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "cpu.h"
 #include <linux/types.h>
 #include "hw/hw.h"
 #include "hw/pci/pci.h"
@@ -26,7 +26,7 @@
 #include "../rdma_utils.h"
 
 #include "pvrdma.h"
-#include <standard-headers/rdma/vmw_pvrdma-abi.h>
+#include "standard-headers/rdma/vmw_pvrdma-abi.h"
 
 static void *pvrdma_map_to_pdir(PCIDevice *pdev, uint64_t pdir_dma,
                                 uint32_t nchunks, size_t length)
diff --git a/hw/rdma/vmw/pvrdma_dev_ring.c b/hw/rdma/vmw/pvrdma_dev_ring.c
index ec309da..ff19a9e 100644
--- a/hw/rdma/vmw/pvrdma_dev_ring.c
+++ b/hw/rdma/vmw/pvrdma_dev_ring.c
@@ -13,12 +13,12 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <hw/pci/pci.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "cpu.h"
 
 #include "../rdma_utils.h"
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h>
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h"
 #include "pvrdma_dev_ring.h"
 
 int pvrdma_ring_init(PvrdmaRing *ring, const char *name, PCIDevice *dev,
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 9978781..25ea02a 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -13,16 +13,16 @@
  *
  */
 
-#include <qemu/osdep.h>
-#include <qapi/error.h>
-#include <hw/hw.h>
-#include <hw/pci/pci.h>
-#include <hw/pci/pci_ids.h>
-#include <hw/pci/msi.h>
-#include <hw/pci/msix.h>
-#include <hw/qdev-core.h>
-#include <hw/qdev-properties.h>
-#include <cpu.h>
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/hw.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_ids.h"
+#include "hw/pci/msi.h"
+#include "hw/pci/msix.h"
+#include "hw/qdev-core.h"
+#include "hw/qdev-properties.h"
+#include "cpu.h"
 #include "trace.h"
 
 #include "../rdma_rm.h"
@@ -31,8 +31,8 @@
 
 #include <infiniband/verbs.h>
 #include "pvrdma.h"
-#include <standard-headers/rdma/vmw_pvrdma-abi.h>
-#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h>
+#include "standard-headers/rdma/vmw_pvrdma-abi.h"
+#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h"
 #include "pvrdma_qp_ops.h"
 
 static Property pvrdma_dev_properties[] = {
diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c
index f0a1f9e..a693c06 100644
--- a/hw/rdma/vmw/pvrdma_qp_ops.c
+++ b/hw/rdma/vmw/pvrdma_qp_ops.c
@@ -13,14 +13,14 @@
  *
  */
 
-#include <qemu/osdep.h>
+#include "qemu/osdep.h"
 
 #include "../rdma_utils.h"
 #include "../rdma_rm.h"
 #include "../rdma_backend.h"
 
 #include "pvrdma.h"
-#include <standard-headers/rdma/vmw_pvrdma-abi.h>
+#include "standard-headers/rdma/vmw_pvrdma-abi.h"
 #include "pvrdma_qp_ops.h"
 
 typedef struct CompHandlerCtx {
-- 
MST

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote
  2018-03-21 15:22 [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives Michael S. Tsirkin
@ 2018-03-21 15:22 ` Michael S. Tsirkin
  2018-03-21 15:36   ` Daniel P. Berrangé
  2018-03-22 21:20   ` Stefan Hajnoczi
  2018-03-21 15:38 ` [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives Daniel P. Berrangé
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2018-03-21 15:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: Yuval Shaia, Marcel Apfelbaum, Paolo Bonzini, Yang Zhong,
	Philippe Mathieu-Daudé,
	Richard Henderson, Peter Maydell, Gerd Hoffmann, Fam Zheng,
	Marc-André Lureau, Stefan Hajnoczi, Daniel P. Berrangé,
	Stefan Weil

Our rule right now is to use <> for external headers,
"" for internal ones. The idea was to avoid conflicts
between e.g. a system file named <trace.h> and an
internal one by the same name.

Unfortunately we use -I compiler flag so it does not
help: a system file doing #include <trace.h> will
still pick up ours first.

To fix, switch to -iquote which is supported by both
gcc and clang and only affects #include "" directives.

As a side effect, this catches any future uses of
 #include <> for internal headers.

Suggested-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

I still think we want to switch to a more formal rule such as qemu/
prefix for all includes down the road, but this will at least catch any
scheme violations from creeping in meanwhile.


 configure       | 16 ++++++++--------
 rules.mak       |  2 +-
 Makefile.target |  4 ++--
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/configure b/configure
index af72fc8..23a4f3b 100755
--- a/configure
+++ b/configure
@@ -534,7 +534,7 @@ QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
 QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
 QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
-QEMU_INCLUDES="-I. -I\$(SRC_PATH) -I\$(SRC_PATH)/accel/tcg -I\$(SRC_PATH)/include"
+QEMU_INCLUDES="-iquote . -iquote \$(SRC_PATH) -iquote \$(SRC_PATH)/accel/tcg -iquote \$(SRC_PATH)/include"
 if test "$debug_info" = "yes"; then
     CFLAGS="-g $CFLAGS"
     LDFLAGS="-g $LDFLAGS"
@@ -6560,19 +6560,19 @@ if test "$vxhs" = "yes" ; then
 fi
 
 if test "$tcg_interpreter" = "yes"; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
 elif test "$ARCH" = "sparc64" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
 elif test "$ARCH" = "s390x" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
 elif test "$ARCH" = "x86_64" -o "$ARCH" = "x32" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
 elif test "$ARCH" = "ppc64" ; then
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
 else
-  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
+  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
 fi
-QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES"
+QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg $QEMU_INCLUDES"
 
 echo "TOOLS=$tools" >> $config_host_mak
 echo "ROMS=$roms" >> $config_host_mak
diff --git a/rules.mak b/rules.mak
index 6e94333..93a0702 100644
--- a/rules.mak
+++ b/rules.mak
@@ -29,7 +29,7 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d
 # dir, one absolute and the other relative to the compiler working
 # directory. These are the same for target-independent files, but
 # different for target-dependent ones.
-QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D) -I$(@D)
+QEMU_LOCAL_INCLUDES = -iquote $(BUILD_DIR)/$(@D) -iquote $(@D)
 
 WL_U := -Wl,-u,
 find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2)))
diff --git a/Makefile.target b/Makefile.target
index 6549481..d0ec77a 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -11,9 +11,9 @@ $(call set-vpath, $(SRC_PATH):$(BUILD_DIR))
 ifdef CONFIG_LINUX
 QEMU_CFLAGS += -I../linux-headers
 endif
-QEMU_CFLAGS += -I.. -I$(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
+QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
 
-QEMU_CFLAGS+=-I$(SRC_PATH)/include
+QEMU_CFLAGS+=-iquote $(SRC_PATH)/include
 
 ifdef CONFIG_USER_ONLY
 # user emulator name
-- 
MST

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote
  2018-03-21 15:22 ` [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote Michael S. Tsirkin
@ 2018-03-21 15:36   ` Daniel P. Berrangé
  2018-03-21 16:02     ` Michael S. Tsirkin
  2018-03-22 21:20   ` Stefan Hajnoczi
  1 sibling, 1 reply; 10+ messages in thread
From: Daniel P. Berrangé @ 2018-03-21 15:36 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: qemu-devel, Yuval Shaia, Marcel Apfelbaum, Paolo Bonzini,
	Yang Zhong, Philippe Mathieu-Daudé,
	Richard Henderson, Peter Maydell, Gerd Hoffmann, Fam Zheng,
	Marc-André Lureau, Stefan Hajnoczi, Stefan Weil

On Wed, Mar 21, 2018 at 05:22:07PM +0200, Michael S. Tsirkin wrote:
> Our rule right now is to use <> for external headers,
> "" for internal ones. The idea was to avoid conflicts
> between e.g. a system file named <trace.h> and an
> internal one by the same name.
> 
> Unfortunately we use -I compiler flag so it does not
> help: a system file doing #include <trace.h> will
> still pick up ours first.

Has that actually hit you practice ?  I'm wondering if this
is just theoretical in which case it could wait till 2.13
since this scenario has existed in QEMU along time, or
a real problem right now requiring fix in 2.12.


> To fix, switch to -iquote which is supported by both
> gcc and clang and only affects #include "" directives.

Fine since we don't support anything other than
gcc and clang. I'm assuming -iquote has been supported
by these two compilers for a long time though ? The GCC
docs annoyingly don't ever mention what release features
appear in :-(

> 
> As a side effect, this catches any future uses of
>  #include <> for internal headers.
> 
> Suggested-by: Stefan Weil <sw@weilnetz.de>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


> 
> I still think we want to switch to a more formal rule such as qemu/
> prefix for all includes down the road, but this will at least catch any
> scheme violations from creeping in meanwhile.
> 
> 
>  configure       | 16 ++++++++--------
>  rules.mak       |  2 +-
>  Makefile.target |  4 ++--
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/configure b/configure
> index af72fc8..23a4f3b 100755
> --- a/configure
> +++ b/configure
> @@ -534,7 +534,7 @@ QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS"
>  QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
>  QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
>  QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
> -QEMU_INCLUDES="-I. -I\$(SRC_PATH) -I\$(SRC_PATH)/accel/tcg -I\$(SRC_PATH)/include"
> +QEMU_INCLUDES="-iquote . -iquote \$(SRC_PATH) -iquote \$(SRC_PATH)/accel/tcg -iquote \$(SRC_PATH)/include"
>  if test "$debug_info" = "yes"; then
>      CFLAGS="-g $CFLAGS"
>      LDFLAGS="-g $LDFLAGS"
> @@ -6560,19 +6560,19 @@ if test "$vxhs" = "yes" ; then
>  fi
>  
>  if test "$tcg_interpreter" = "yes"; then
> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
>  elif test "$ARCH" = "sparc64" ; then
> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
>  elif test "$ARCH" = "s390x" ; then
> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
>  elif test "$ARCH" = "x86_64" -o "$ARCH" = "x32" ; then
> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
>  elif test "$ARCH" = "ppc64" ; then
> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
>  else
> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
>  fi
> -QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES"
> +QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg $QEMU_INCLUDES"
>  
>  echo "TOOLS=$tools" >> $config_host_mak
>  echo "ROMS=$roms" >> $config_host_mak
> diff --git a/rules.mak b/rules.mak
> index 6e94333..93a0702 100644
> --- a/rules.mak
> +++ b/rules.mak
> @@ -29,7 +29,7 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d
>  # dir, one absolute and the other relative to the compiler working
>  # directory. These are the same for target-independent files, but
>  # different for target-dependent ones.
> -QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D) -I$(@D)
> +QEMU_LOCAL_INCLUDES = -iquote $(BUILD_DIR)/$(@D) -iquote $(@D)
>  
>  WL_U := -Wl,-u,
>  find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2)))
> diff --git a/Makefile.target b/Makefile.target
> index 6549481..d0ec77a 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -11,9 +11,9 @@ $(call set-vpath, $(SRC_PATH):$(BUILD_DIR))
>  ifdef CONFIG_LINUX
>  QEMU_CFLAGS += -I../linux-headers
>  endif
> -QEMU_CFLAGS += -I.. -I$(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
> +QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
>  
> -QEMU_CFLAGS+=-I$(SRC_PATH)/include
> +QEMU_CFLAGS+=-iquote $(SRC_PATH)/include
>  
>  ifdef CONFIG_USER_ONLY
>  # user emulator name
> -- 
> MST
> 

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives
  2018-03-21 15:22 [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives Michael S. Tsirkin
  2018-03-21 15:22 ` [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote Michael S. Tsirkin
@ 2018-03-21 15:38 ` Daniel P. Berrangé
  2018-03-21 16:35 ` Marcel Apfelbaum
  2018-03-21 21:15 ` Yuval Shaia
  3 siblings, 0 replies; 10+ messages in thread
From: Daniel P. Berrangé @ 2018-03-21 15:38 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, Marcel Apfelbaum, Yuval Shaia

On Wed, Mar 21, 2018 at 05:22:07PM +0200, Michael S. Tsirkin wrote:
> Our rule right now is to use <> for external headers only.
> RDMA code violates that, fix it up.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  hw/rdma/rdma_backend.h        |  2 +-
>  hw/rdma/rdma_backend_defs.h   |  2 +-
>  hw/rdma/rdma_rm.h             |  2 +-
>  hw/rdma/rdma_utils.h          |  6 +++---
>  hw/rdma/vmw/pvrdma.h          |  8 ++++----
>  hw/rdma/vmw/pvrdma_dev_ring.h |  2 +-
>  hw/rdma/rdma_backend.c        |  6 +++---
>  hw/rdma/rdma_rm.c             |  6 +++---
>  hw/rdma/vmw/pvrdma_cmd.c      |  8 ++++----
>  hw/rdma/vmw/pvrdma_dev_ring.c |  8 ++++----
>  hw/rdma/vmw/pvrdma_main.c     | 24 ++++++++++++------------
>  hw/rdma/vmw/pvrdma_qp_ops.c   |  4 ++--
>  12 files changed, 39 insertions(+), 39 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


I'm assuming these were detected by build failures caused by the 2nd
patch in this series, so other areas of the codebase are all correct
already.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote
  2018-03-21 15:36   ` Daniel P. Berrangé
@ 2018-03-21 16:02     ` Michael S. Tsirkin
  2018-03-21 16:40       ` Marcel Apfelbaum
  0 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2018-03-21 16:02 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: qemu-devel, Yuval Shaia, Marcel Apfelbaum, Paolo Bonzini,
	Yang Zhong, Philippe Mathieu-Daudé,
	Richard Henderson, Peter Maydell, Gerd Hoffmann, Fam Zheng,
	Marc-André Lureau, Stefan Hajnoczi, Stefan Weil

On Wed, Mar 21, 2018 at 03:36:51PM +0000, Daniel P. Berrangé wrote:
> On Wed, Mar 21, 2018 at 05:22:07PM +0200, Michael S. Tsirkin wrote:
> > Our rule right now is to use <> for external headers,
> > "" for internal ones. The idea was to avoid conflicts
> > between e.g. a system file named <trace.h> and an
> > internal one by the same name.
> > 
> > Unfortunately we use -I compiler flag so it does not
> > help: a system file doing #include <trace.h> will
> > still pick up ours first.
> 
> Has that actually hit you practice ?  I'm wondering if this
> is just theoretical in which case it could wait till 2.13
> since this scenario has existed in QEMU along time, or
> a real problem right now requiring fix in 2.12.

It's borderline.

In particular there is at least one instance where we already have a
conflict (yes, there are probably other ways to address this, but
I consider them less robust):

util/getauxval.c:#include <sys/auxv.h>
/usr/include/sys/auxv.h:#include <elf.h>

So it really does build by luck.


> > To fix, switch to -iquote which is supported by both
> > gcc and clang and only affects #include "" directives.
> 
> Fine since we don't support anything other than
> gcc and clang. I'm assuming -iquote has been supported
> by these two compilers for a long time though ? The GCC
> docs annoyingly don't ever mention what release features
> appear in :-(

Judging by git log output:

For clang it appears to be there since 2007 that's pretty
close to day 1.

gcc's log is better documented, it's been there since 2004-05-03

> > 
> > As a side effect, this catches any future uses of
> >  #include <> for internal headers.
> > 
> > Suggested-by: Stefan Weil <sw@weilnetz.de>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> 
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Thanks!

> 
> > 
> > I still think we want to switch to a more formal rule such as qemu/
> > prefix for all includes down the road, but this will at least catch any
> > scheme violations from creeping in meanwhile.
> > 
> > 
> >  configure       | 16 ++++++++--------
> >  rules.mak       |  2 +-
> >  Makefile.target |  4 ++--
> >  3 files changed, 11 insertions(+), 11 deletions(-)
> > 
> > diff --git a/configure b/configure
> > index af72fc8..23a4f3b 100755
> > --- a/configure
> > +++ b/configure
> > @@ -534,7 +534,7 @@ QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS"
> >  QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
> >  QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
> >  QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
> > -QEMU_INCLUDES="-I. -I\$(SRC_PATH) -I\$(SRC_PATH)/accel/tcg -I\$(SRC_PATH)/include"
> > +QEMU_INCLUDES="-iquote . -iquote \$(SRC_PATH) -iquote \$(SRC_PATH)/accel/tcg -iquote \$(SRC_PATH)/include"
> >  if test "$debug_info" = "yes"; then
> >      CFLAGS="-g $CFLAGS"
> >      LDFLAGS="-g $LDFLAGS"
> > @@ -6560,19 +6560,19 @@ if test "$vxhs" = "yes" ; then
> >  fi
> >  
> >  if test "$tcg_interpreter" = "yes"; then
> > -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
> > +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
> >  elif test "$ARCH" = "sparc64" ; then
> > -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
> > +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
> >  elif test "$ARCH" = "s390x" ; then
> > -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
> > +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
> >  elif test "$ARCH" = "x86_64" -o "$ARCH" = "x32" ; then
> > -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
> > +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
> >  elif test "$ARCH" = "ppc64" ; then
> > -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
> > +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
> >  else
> > -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
> > +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
> >  fi
> > -QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES"
> > +QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg $QEMU_INCLUDES"
> >  
> >  echo "TOOLS=$tools" >> $config_host_mak
> >  echo "ROMS=$roms" >> $config_host_mak
> > diff --git a/rules.mak b/rules.mak
> > index 6e94333..93a0702 100644
> > --- a/rules.mak
> > +++ b/rules.mak
> > @@ -29,7 +29,7 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d
> >  # dir, one absolute and the other relative to the compiler working
> >  # directory. These are the same for target-independent files, but
> >  # different for target-dependent ones.
> > -QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D) -I$(@D)
> > +QEMU_LOCAL_INCLUDES = -iquote $(BUILD_DIR)/$(@D) -iquote $(@D)
> >  
> >  WL_U := -Wl,-u,
> >  find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2)))
> > diff --git a/Makefile.target b/Makefile.target
> > index 6549481..d0ec77a 100644
> > --- a/Makefile.target
> > +++ b/Makefile.target
> > @@ -11,9 +11,9 @@ $(call set-vpath, $(SRC_PATH):$(BUILD_DIR))
> >  ifdef CONFIG_LINUX
> >  QEMU_CFLAGS += -I../linux-headers
> >  endif
> > -QEMU_CFLAGS += -I.. -I$(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
> > +QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
> >  
> > -QEMU_CFLAGS+=-I$(SRC_PATH)/include
> > +QEMU_CFLAGS+=-iquote $(SRC_PATH)/include
> >  
> >  ifdef CONFIG_USER_ONLY
> >  # user emulator name
> > -- 
> > MST
> > 
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives
  2018-03-21 15:22 [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives Michael S. Tsirkin
  2018-03-21 15:22 ` [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote Michael S. Tsirkin
  2018-03-21 15:38 ` [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives Daniel P. Berrangé
@ 2018-03-21 16:35 ` Marcel Apfelbaum
  2018-03-21 21:15 ` Yuval Shaia
  3 siblings, 0 replies; 10+ messages in thread
From: Marcel Apfelbaum @ 2018-03-21 16:35 UTC (permalink / raw)
  To: Michael S. Tsirkin, qemu-devel; +Cc: Yuval Shaia

On 21/03/2018 17:22, Michael S. Tsirkin wrote:
> Our rule right now is to use <> for external headers only.
> RDMA code violates that, fix it up.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>

Thanks,
Marcel

> ---
>  hw/rdma/rdma_backend.h        |  2 +-
>  hw/rdma/rdma_backend_defs.h   |  2 +-
>  hw/rdma/rdma_rm.h             |  2 +-
>  hw/rdma/rdma_utils.h          |  6 +++---
>  hw/rdma/vmw/pvrdma.h          |  8 ++++----
>  hw/rdma/vmw/pvrdma_dev_ring.h |  2 +-
>  hw/rdma/rdma_backend.c        |  6 +++---
>  hw/rdma/rdma_rm.c             |  6 +++---
>  hw/rdma/vmw/pvrdma_cmd.c      |  8 ++++----
>  hw/rdma/vmw/pvrdma_dev_ring.c |  8 ++++----
>  hw/rdma/vmw/pvrdma_main.c     | 24 ++++++++++++------------
>  hw/rdma/vmw/pvrdma_qp_ops.c   |  4 ++--
>  12 files changed, 39 insertions(+), 39 deletions(-)
> 
> diff --git a/hw/rdma/rdma_backend.h b/hw/rdma/rdma_backend.h
> index 68f2b05..abae262 100644
> --- a/hw/rdma/rdma_backend.h
> +++ b/hw/rdma/rdma_backend.h
> @@ -16,7 +16,7 @@
>  #ifndef RDMA_BACKEND_H
>  #define RDMA_BACKEND_H
>  
> -#include <qapi/error.h>
> +#include "qapi/error.h"
>  #include "rdma_rm_defs.h"
>  #include "rdma_backend_defs.h"
>  
> diff --git a/hw/rdma/rdma_backend_defs.h b/hw/rdma/rdma_backend_defs.h
> index 837e324..ff5cfc2 100644
> --- a/hw/rdma/rdma_backend_defs.h
> +++ b/hw/rdma/rdma_backend_defs.h
> @@ -17,7 +17,7 @@
>  #define RDMA_BACKEND_DEFS_H
>  
>  #include <infiniband/verbs.h>
> -#include <qemu/thread.h>
> +#include "qemu/thread.h"
>  
>  typedef struct RdmaDeviceResources RdmaDeviceResources;
>  
> diff --git a/hw/rdma/rdma_rm.h b/hw/rdma/rdma_rm.h
> index be95c1b..82c1ddd 100644
> --- a/hw/rdma/rdma_rm.h
> +++ b/hw/rdma/rdma_rm.h
> @@ -16,7 +16,7 @@
>  #ifndef RDMA_RM_H
>  #define RDMA_RM_H
>  
> -#include <qapi/error.h>
> +#include "qapi/error.h"
>  #include "rdma_backend_defs.h"
>  #include "rdma_rm_defs.h"
>  
> diff --git a/hw/rdma/rdma_utils.h b/hw/rdma/rdma_utils.h
> index cdac910..3dc0789 100644
> --- a/hw/rdma/rdma_utils.h
> +++ b/hw/rdma/rdma_utils.h
> @@ -17,9 +17,9 @@
>  #ifndef RDMA_UTILS_H
>  #define RDMA_UTILS_H
>  
> -#include <qemu/osdep.h>
> -#include <include/hw/pci/pci.h>
> -#include <include/sysemu/dma.h>
> +#include "qemu/osdep.h"
> +#include "hw/pci/pci.h"
> +#include "sysemu/dma.h"
>  
>  #define pr_info(fmt, ...) \
>      fprintf(stdout, "%s: %-20s (%3d): " fmt, "pvrdma",  __func__, __LINE__,\
> diff --git a/hw/rdma/vmw/pvrdma.h b/hw/rdma/vmw/pvrdma.h
> index b05f94a..8c173cb 100644
> --- a/hw/rdma/vmw/pvrdma.h
> +++ b/hw/rdma/vmw/pvrdma.h
> @@ -16,14 +16,14 @@
>  #ifndef PVRDMA_PVRDMA_H
>  #define PVRDMA_PVRDMA_H
>  
> -#include <hw/pci/pci.h>
> -#include <hw/pci/msix.h>
> +#include "hw/pci/pci.h"
> +#include "hw/pci/msix.h"
>  
>  #include "../rdma_backend_defs.h"
>  #include "../rdma_rm_defs.h"
>  
> -#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h>
> -#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h>
> +#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h"
> +#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h"
>  #include "pvrdma_dev_ring.h"
>  
>  /* BARs */
> diff --git a/hw/rdma/vmw/pvrdma_dev_ring.h b/hw/rdma/vmw/pvrdma_dev_ring.h
> index 02a590b..2d0461f 100644
> --- a/hw/rdma/vmw/pvrdma_dev_ring.h
> +++ b/hw/rdma/vmw/pvrdma_dev_ring.h
> @@ -16,7 +16,7 @@
>  #ifndef PVRDMA_DEV_RING_H
>  #define PVRDMA_DEV_RING_H
>  
> -#include <qemu/typedefs.h>
> +#include "qemu/typedefs.h"
>  
>  #define MAX_RING_NAME_SZ 32
>  
> diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
> index e306fba..f85c7b2 100644
> --- a/hw/rdma/rdma_backend.c
> +++ b/hw/rdma/rdma_backend.c
> @@ -13,9 +13,9 @@
>   *
>   */
>  
> -#include <qemu/osdep.h>
> -#include <qemu/error-report.h>
> -#include <qapi/error.h>
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "qapi/error.h"
>  
>  #include <infiniband/verbs.h>
>  
> diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
> index b5fc45d..80ffc94 100644
> --- a/hw/rdma/rdma_rm.c
> +++ b/hw/rdma/rdma_rm.c
> @@ -13,9 +13,9 @@
>   *
>   */
>  
> -#include <qemu/osdep.h>
> -#include <qapi/error.h>
> -#include <cpu.h>
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "cpu.h"
>  
>  #include "rdma_utils.h"
>  #include "rdma_backend.h"
> diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
> index 293dfed..a9fa1ef 100644
> --- a/hw/rdma/vmw/pvrdma_cmd.c
> +++ b/hw/rdma/vmw/pvrdma_cmd.c
> @@ -13,9 +13,9 @@
>   *
>   */
>  
> -#include <qemu/osdep.h>
> -#include <qemu/error-report.h>
> -#include <cpu.h>
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "cpu.h"
>  #include <linux/types.h>
>  #include "hw/hw.h"
>  #include "hw/pci/pci.h"
> @@ -26,7 +26,7 @@
>  #include "../rdma_utils.h"
>  
>  #include "pvrdma.h"
> -#include <standard-headers/rdma/vmw_pvrdma-abi.h>
> +#include "standard-headers/rdma/vmw_pvrdma-abi.h"
>  
>  static void *pvrdma_map_to_pdir(PCIDevice *pdev, uint64_t pdir_dma,
>                                  uint32_t nchunks, size_t length)
> diff --git a/hw/rdma/vmw/pvrdma_dev_ring.c b/hw/rdma/vmw/pvrdma_dev_ring.c
> index ec309da..ff19a9e 100644
> --- a/hw/rdma/vmw/pvrdma_dev_ring.c
> +++ b/hw/rdma/vmw/pvrdma_dev_ring.c
> @@ -13,12 +13,12 @@
>   *
>   */
>  
> -#include <qemu/osdep.h>
> -#include <hw/pci/pci.h>
> -#include <cpu.h>
> +#include "qemu/osdep.h"
> +#include "hw/pci/pci.h"
> +#include "cpu.h"
>  
>  #include "../rdma_utils.h"
> -#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h>
> +#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h"
>  #include "pvrdma_dev_ring.h"
>  
>  int pvrdma_ring_init(PvrdmaRing *ring, const char *name, PCIDevice *dev,
> diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> index 9978781..25ea02a 100644
> --- a/hw/rdma/vmw/pvrdma_main.c
> +++ b/hw/rdma/vmw/pvrdma_main.c
> @@ -13,16 +13,16 @@
>   *
>   */
>  
> -#include <qemu/osdep.h>
> -#include <qapi/error.h>
> -#include <hw/hw.h>
> -#include <hw/pci/pci.h>
> -#include <hw/pci/pci_ids.h>
> -#include <hw/pci/msi.h>
> -#include <hw/pci/msix.h>
> -#include <hw/qdev-core.h>
> -#include <hw/qdev-properties.h>
> -#include <cpu.h>
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/hw.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pci_ids.h"
> +#include "hw/pci/msi.h"
> +#include "hw/pci/msix.h"
> +#include "hw/qdev-core.h"
> +#include "hw/qdev-properties.h"
> +#include "cpu.h"
>  #include "trace.h"
>  
>  #include "../rdma_rm.h"
> @@ -31,8 +31,8 @@
>  
>  #include <infiniband/verbs.h>
>  #include "pvrdma.h"
> -#include <standard-headers/rdma/vmw_pvrdma-abi.h>
> -#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h>
> +#include "standard-headers/rdma/vmw_pvrdma-abi.h"
> +#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h"
>  #include "pvrdma_qp_ops.h"
>  
>  static Property pvrdma_dev_properties[] = {
> diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c
> index f0a1f9e..a693c06 100644
> --- a/hw/rdma/vmw/pvrdma_qp_ops.c
> +++ b/hw/rdma/vmw/pvrdma_qp_ops.c
> @@ -13,14 +13,14 @@
>   *
>   */
>  
> -#include <qemu/osdep.h>
> +#include "qemu/osdep.h"
>  
>  #include "../rdma_utils.h"
>  #include "../rdma_rm.h"
>  #include "../rdma_backend.h"
>  
>  #include "pvrdma.h"
> -#include <standard-headers/rdma/vmw_pvrdma-abi.h>
> +#include "standard-headers/rdma/vmw_pvrdma-abi.h"
>  #include "pvrdma_qp_ops.h"
>  
>  typedef struct CompHandlerCtx {
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote
  2018-03-21 16:02     ` Michael S. Tsirkin
@ 2018-03-21 16:40       ` Marcel Apfelbaum
  2018-03-21 19:13         ` Michael S. Tsirkin
  0 siblings, 1 reply; 10+ messages in thread
From: Marcel Apfelbaum @ 2018-03-21 16:40 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Daniel P. Berrangé,
	qemu-devel, Yuval Shaia, Paolo Bonzini, Yang Zhong,
	Philippe Mathieu-Daudé,
	Richard Henderson, Peter Maydell, Gerd Hoffmann, Fam Zheng,
	Marc-André Lureau, Stefan Hajnoczi, Stefan Weil

On 21/03/2018 18:02, Michael S. Tsirkin wrote:
> On Wed, Mar 21, 2018 at 03:36:51PM +0000, Daniel P. Berrangé wrote:
>> On Wed, Mar 21, 2018 at 05:22:07PM +0200, Michael S. Tsirkin wrote:
>>> Our rule right now is to use <> for external headers,
>>> "" for internal ones. The idea was to avoid conflicts
>>> between e.g. a system file named <trace.h> and an
>>> internal one by the same name.
>>>
>>> Unfortunately we use -I compiler flag so it does not
>>> help: a system file doing #include <trace.h> will
>>> still pick up ours first.
>>
>> Has that actually hit you practice ?  I'm wondering if this
>> is just theoretical in which case it could wait till 2.13
>> since this scenario has existed in QEMU along time, or
>> a real problem right now requiring fix in 2.12.
> 
> It's borderline.
> 
> In particular there is at least one instance where we already have a
> conflict (yes, there are probably other ways to address this, but
> I consider them less robust):
> 
> util/getauxval.c:#include <sys/auxv.h>
> /usr/include/sys/auxv.h:#include <elf.h>
> 
> So it really does build by luck.
> 
> 
>>> To fix, switch to -iquote which is supported by both
>>> gcc and clang and only affects #include "" directives.
>>
>> Fine since we don't support anything other than
>> gcc and clang. I'm assuming -iquote has been supported
>> by these two compilers for a long time though ? The GCC
>> docs annoyingly don't ever mention what release features
>> appear in :-(
> 
> Judging by git log output:
> 
> For clang it appears to be there since 2007 that's pretty
> close to day 1.
> 
> gcc's log is better documented, it's been there since 2004-05-03
> 
>>>
>>> As a side effect, this catches any future uses of
>>>  #include <> for internal headers.
>>>
>>> Suggested-by: Stefan Weil <sw@weilnetz.de>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> 
> Thanks!
> 

Hi Michael,
Should I add this patch to the RDMA pull request?

Thanks,
Marcel


>>
>>>
>>> I still think we want to switch to a more formal rule such as qemu/
>>> prefix for all includes down the road, but this will at least catch any
>>> scheme violations from creeping in meanwhile.
>>>
>>>
>>>  configure       | 16 ++++++++--------
>>>  rules.mak       |  2 +-
>>>  Makefile.target |  4 ++--
>>>  3 files changed, 11 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/configure b/configure
>>> index af72fc8..23a4f3b 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -534,7 +534,7 @@ QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS"
>>>  QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
>>>  QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
>>>  QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
>>> -QEMU_INCLUDES="-I. -I\$(SRC_PATH) -I\$(SRC_PATH)/accel/tcg -I\$(SRC_PATH)/include"
>>> +QEMU_INCLUDES="-iquote . -iquote \$(SRC_PATH) -iquote \$(SRC_PATH)/accel/tcg -iquote \$(SRC_PATH)/include"
>>>  if test "$debug_info" = "yes"; then
>>>      CFLAGS="-g $CFLAGS"
>>>      LDFLAGS="-g $LDFLAGS"
>>> @@ -6560,19 +6560,19 @@ if test "$vxhs" = "yes" ; then
>>>  fi
>>>  
>>>  if test "$tcg_interpreter" = "yes"; then
>>> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
>>> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
>>>  elif test "$ARCH" = "sparc64" ; then
>>> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
>>> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
>>>  elif test "$ARCH" = "s390x" ; then
>>> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
>>> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
>>>  elif test "$ARCH" = "x86_64" -o "$ARCH" = "x32" ; then
>>> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
>>> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
>>>  elif test "$ARCH" = "ppc64" ; then
>>> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
>>> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
>>>  else
>>> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
>>> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
>>>  fi
>>> -QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES"
>>> +QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg $QEMU_INCLUDES"
>>>  
>>>  echo "TOOLS=$tools" >> $config_host_mak
>>>  echo "ROMS=$roms" >> $config_host_mak
>>> diff --git a/rules.mak b/rules.mak
>>> index 6e94333..93a0702 100644
>>> --- a/rules.mak
>>> +++ b/rules.mak
>>> @@ -29,7 +29,7 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d
>>>  # dir, one absolute and the other relative to the compiler working
>>>  # directory. These are the same for target-independent files, but
>>>  # different for target-dependent ones.
>>> -QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D) -I$(@D)
>>> +QEMU_LOCAL_INCLUDES = -iquote $(BUILD_DIR)/$(@D) -iquote $(@D)
>>>  
>>>  WL_U := -Wl,-u,
>>>  find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2)))
>>> diff --git a/Makefile.target b/Makefile.target
>>> index 6549481..d0ec77a 100644
>>> --- a/Makefile.target
>>> +++ b/Makefile.target
>>> @@ -11,9 +11,9 @@ $(call set-vpath, $(SRC_PATH):$(BUILD_DIR))
>>>  ifdef CONFIG_LINUX
>>>  QEMU_CFLAGS += -I../linux-headers
>>>  endif
>>> -QEMU_CFLAGS += -I.. -I$(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
>>> +QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
>>>  
>>> -QEMU_CFLAGS+=-I$(SRC_PATH)/include
>>> +QEMU_CFLAGS+=-iquote $(SRC_PATH)/include
>>>  
>>>  ifdef CONFIG_USER_ONLY
>>>  # user emulator name
>>> -- 
>>> MST
>>>
>>
>> Regards,
>> Daniel
>> -- 
>> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote
  2018-03-21 16:40       ` Marcel Apfelbaum
@ 2018-03-21 19:13         ` Michael S. Tsirkin
  0 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2018-03-21 19:13 UTC (permalink / raw)
  To: Marcel Apfelbaum
  Cc: Daniel P. Berrangé,
	qemu-devel, Yuval Shaia, Paolo Bonzini, Yang Zhong,
	Philippe Mathieu-Daudé,
	Richard Henderson, Peter Maydell, Gerd Hoffmann, Fam Zheng,
	Marc-André Lureau, Stefan Hajnoczi, Stefan Weil

On Wed, Mar 21, 2018 at 06:40:23PM +0200, Marcel Apfelbaum wrote:
> On 21/03/2018 18:02, Michael S. Tsirkin wrote:
> > On Wed, Mar 21, 2018 at 03:36:51PM +0000, Daniel P. Berrangé wrote:
> >> On Wed, Mar 21, 2018 at 05:22:07PM +0200, Michael S. Tsirkin wrote:
> >>> Our rule right now is to use <> for external headers,
> >>> "" for internal ones. The idea was to avoid conflicts
> >>> between e.g. a system file named <trace.h> and an
> >>> internal one by the same name.
> >>>
> >>> Unfortunately we use -I compiler flag so it does not
> >>> help: a system file doing #include <trace.h> will
> >>> still pick up ours first.
> >>
> >> Has that actually hit you practice ?  I'm wondering if this
> >> is just theoretical in which case it could wait till 2.13
> >> since this scenario has existed in QEMU along time, or
> >> a real problem right now requiring fix in 2.12.
> > 
> > It's borderline.
> > 
> > In particular there is at least one instance where we already have a
> > conflict (yes, there are probably other ways to address this, but
> > I consider them less robust):
> > 
> > util/getauxval.c:#include <sys/auxv.h>
> > /usr/include/sys/auxv.h:#include <elf.h>
> > 
> > So it really does build by luck.
> > 
> > 
> >>> To fix, switch to -iquote which is supported by both
> >>> gcc and clang and only affects #include "" directives.
> >>
> >> Fine since we don't support anything other than
> >> gcc and clang. I'm assuming -iquote has been supported
> >> by these two compilers for a long time though ? The GCC
> >> docs annoyingly don't ever mention what release features
> >> appear in :-(
> > 
> > Judging by git log output:
> > 
> > For clang it appears to be there since 2007 that's pretty
> > close to day 1.
> > 
> > gcc's log is better documented, it's been there since 2004-05-03
> > 
> >>>
> >>> As a side effect, this catches any future uses of
> >>>  #include <> for internal headers.
> >>>
> >>> Suggested-by: Stefan Weil <sw@weilnetz.de>
> >>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >>> ---
> >>
> >> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > 
> > Thanks!
> > 
> 
> Hi Michael,
> Should I add this patch to the RDMA pull request?
> 
> Thanks,
> Marcel

Go ahead and add both of them pls.

> 
> >>
> >>>
> >>> I still think we want to switch to a more formal rule such as qemu/
> >>> prefix for all includes down the road, but this will at least catch any
> >>> scheme violations from creeping in meanwhile.
> >>>
> >>>
> >>>  configure       | 16 ++++++++--------
> >>>  rules.mak       |  2 +-
> >>>  Makefile.target |  4 ++--
> >>>  3 files changed, 11 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/configure b/configure
> >>> index af72fc8..23a4f3b 100755
> >>> --- a/configure
> >>> +++ b/configure
> >>> @@ -534,7 +534,7 @@ QEMU_CFLAGS="-fno-strict-aliasing -fno-common -fwrapv $QEMU_CFLAGS"
> >>>  QEMU_CFLAGS="-Wall -Wundef -Wwrite-strings -Wmissing-prototypes $QEMU_CFLAGS"
> >>>  QEMU_CFLAGS="-Wstrict-prototypes -Wredundant-decls $QEMU_CFLAGS"
> >>>  QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE $QEMU_CFLAGS"
> >>> -QEMU_INCLUDES="-I. -I\$(SRC_PATH) -I\$(SRC_PATH)/accel/tcg -I\$(SRC_PATH)/include"
> >>> +QEMU_INCLUDES="-iquote . -iquote \$(SRC_PATH) -iquote \$(SRC_PATH)/accel/tcg -iquote \$(SRC_PATH)/include"
> >>>  if test "$debug_info" = "yes"; then
> >>>      CFLAGS="-g $CFLAGS"
> >>>      LDFLAGS="-g $LDFLAGS"
> >>> @@ -6560,19 +6560,19 @@ if test "$vxhs" = "yes" ; then
> >>>  fi
> >>>  
> >>>  if test "$tcg_interpreter" = "yes"; then
> >>> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
> >>> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
> >>>  elif test "$ARCH" = "sparc64" ; then
> >>> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
> >>> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/sparc $QEMU_INCLUDES"
> >>>  elif test "$ARCH" = "s390x" ; then
> >>> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
> >>> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/s390 $QEMU_INCLUDES"
> >>>  elif test "$ARCH" = "x86_64" -o "$ARCH" = "x32" ; then
> >>> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
> >>> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/i386 $QEMU_INCLUDES"
> >>>  elif test "$ARCH" = "ppc64" ; then
> >>> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
> >>> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/ppc $QEMU_INCLUDES"
> >>>  else
> >>> -  QEMU_INCLUDES="-I\$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
> >>> +  QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES"
> >>>  fi
> >>> -QEMU_INCLUDES="-I\$(SRC_PATH)/tcg $QEMU_INCLUDES"
> >>> +QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg $QEMU_INCLUDES"
> >>>  
> >>>  echo "TOOLS=$tools" >> $config_host_mak
> >>>  echo "ROMS=$roms" >> $config_host_mak
> >>> diff --git a/rules.mak b/rules.mak
> >>> index 6e94333..93a0702 100644
> >>> --- a/rules.mak
> >>> +++ b/rules.mak
> >>> @@ -29,7 +29,7 @@ QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d
> >>>  # dir, one absolute and the other relative to the compiler working
> >>>  # directory. These are the same for target-independent files, but
> >>>  # different for target-dependent ones.
> >>> -QEMU_LOCAL_INCLUDES = -I$(BUILD_DIR)/$(@D) -I$(@D)
> >>> +QEMU_LOCAL_INCLUDES = -iquote $(BUILD_DIR)/$(@D) -iquote $(@D)
> >>>  
> >>>  WL_U := -Wl,-u,
> >>>  find-symbols = $(if $1, $(sort $(shell $(NM) -P -g $1 | $2)))
> >>> diff --git a/Makefile.target b/Makefile.target
> >>> index 6549481..d0ec77a 100644
> >>> --- a/Makefile.target
> >>> +++ b/Makefile.target
> >>> @@ -11,9 +11,9 @@ $(call set-vpath, $(SRC_PATH):$(BUILD_DIR))
> >>>  ifdef CONFIG_LINUX
> >>>  QEMU_CFLAGS += -I../linux-headers
> >>>  endif
> >>> -QEMU_CFLAGS += -I.. -I$(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
> >>> +QEMU_CFLAGS += -iquote .. -iquote $(SRC_PATH)/target/$(TARGET_BASE_ARCH) -DNEED_CPU_H
> >>>  
> >>> -QEMU_CFLAGS+=-I$(SRC_PATH)/include
> >>> +QEMU_CFLAGS+=-iquote $(SRC_PATH)/include
> >>>  
> >>>  ifdef CONFIG_USER_ONLY
> >>>  # user emulator name
> >>> -- 
> >>> MST
> >>>
> >>
> >> Regards,
> >> Daniel
> >> -- 
> >> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> >> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> >> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives
  2018-03-21 15:22 [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives Michael S. Tsirkin
                   ` (2 preceding siblings ...)
  2018-03-21 16:35 ` Marcel Apfelbaum
@ 2018-03-21 21:15 ` Yuval Shaia
  3 siblings, 0 replies; 10+ messages in thread
From: Yuval Shaia @ 2018-03-21 21:15 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel, Marcel Apfelbaum

On Wed, Mar 21, 2018 at 05:22:07PM +0200, Michael S. Tsirkin wrote:
> Our rule right now is to use <> for external headers only.
> RDMA code violates that, fix it up.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Thanks.

Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>

> ---
>  hw/rdma/rdma_backend.h        |  2 +-
>  hw/rdma/rdma_backend_defs.h   |  2 +-
>  hw/rdma/rdma_rm.h             |  2 +-
>  hw/rdma/rdma_utils.h          |  6 +++---
>  hw/rdma/vmw/pvrdma.h          |  8 ++++----
>  hw/rdma/vmw/pvrdma_dev_ring.h |  2 +-
>  hw/rdma/rdma_backend.c        |  6 +++---
>  hw/rdma/rdma_rm.c             |  6 +++---
>  hw/rdma/vmw/pvrdma_cmd.c      |  8 ++++----
>  hw/rdma/vmw/pvrdma_dev_ring.c |  8 ++++----
>  hw/rdma/vmw/pvrdma_main.c     | 24 ++++++++++++------------
>  hw/rdma/vmw/pvrdma_qp_ops.c   |  4 ++--
>  12 files changed, 39 insertions(+), 39 deletions(-)
> 
> diff --git a/hw/rdma/rdma_backend.h b/hw/rdma/rdma_backend.h
> index 68f2b05..abae262 100644
> --- a/hw/rdma/rdma_backend.h
> +++ b/hw/rdma/rdma_backend.h
> @@ -16,7 +16,7 @@
>  #ifndef RDMA_BACKEND_H
>  #define RDMA_BACKEND_H
>  
> -#include <qapi/error.h>
> +#include "qapi/error.h"
>  #include "rdma_rm_defs.h"
>  #include "rdma_backend_defs.h"
>  
> diff --git a/hw/rdma/rdma_backend_defs.h b/hw/rdma/rdma_backend_defs.h
> index 837e324..ff5cfc2 100644
> --- a/hw/rdma/rdma_backend_defs.h
> +++ b/hw/rdma/rdma_backend_defs.h
> @@ -17,7 +17,7 @@
>  #define RDMA_BACKEND_DEFS_H
>  
>  #include <infiniband/verbs.h>
> -#include <qemu/thread.h>
> +#include "qemu/thread.h"
>  
>  typedef struct RdmaDeviceResources RdmaDeviceResources;
>  
> diff --git a/hw/rdma/rdma_rm.h b/hw/rdma/rdma_rm.h
> index be95c1b..82c1ddd 100644
> --- a/hw/rdma/rdma_rm.h
> +++ b/hw/rdma/rdma_rm.h
> @@ -16,7 +16,7 @@
>  #ifndef RDMA_RM_H
>  #define RDMA_RM_H
>  
> -#include <qapi/error.h>
> +#include "qapi/error.h"
>  #include "rdma_backend_defs.h"
>  #include "rdma_rm_defs.h"
>  
> diff --git a/hw/rdma/rdma_utils.h b/hw/rdma/rdma_utils.h
> index cdac910..3dc0789 100644
> --- a/hw/rdma/rdma_utils.h
> +++ b/hw/rdma/rdma_utils.h
> @@ -17,9 +17,9 @@
>  #ifndef RDMA_UTILS_H
>  #define RDMA_UTILS_H
>  
> -#include <qemu/osdep.h>
> -#include <include/hw/pci/pci.h>
> -#include <include/sysemu/dma.h>
> +#include "qemu/osdep.h"
> +#include "hw/pci/pci.h"
> +#include "sysemu/dma.h"
>  
>  #define pr_info(fmt, ...) \
>      fprintf(stdout, "%s: %-20s (%3d): " fmt, "pvrdma",  __func__, __LINE__,\
> diff --git a/hw/rdma/vmw/pvrdma.h b/hw/rdma/vmw/pvrdma.h
> index b05f94a..8c173cb 100644
> --- a/hw/rdma/vmw/pvrdma.h
> +++ b/hw/rdma/vmw/pvrdma.h
> @@ -16,14 +16,14 @@
>  #ifndef PVRDMA_PVRDMA_H
>  #define PVRDMA_PVRDMA_H
>  
> -#include <hw/pci/pci.h>
> -#include <hw/pci/msix.h>
> +#include "hw/pci/pci.h"
> +#include "hw/pci/msix.h"
>  
>  #include "../rdma_backend_defs.h"
>  #include "../rdma_rm_defs.h"
>  
> -#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h>
> -#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h>
> +#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h"
> +#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h"
>  #include "pvrdma_dev_ring.h"
>  
>  /* BARs */
> diff --git a/hw/rdma/vmw/pvrdma_dev_ring.h b/hw/rdma/vmw/pvrdma_dev_ring.h
> index 02a590b..2d0461f 100644
> --- a/hw/rdma/vmw/pvrdma_dev_ring.h
> +++ b/hw/rdma/vmw/pvrdma_dev_ring.h
> @@ -16,7 +16,7 @@
>  #ifndef PVRDMA_DEV_RING_H
>  #define PVRDMA_DEV_RING_H
>  
> -#include <qemu/typedefs.h>
> +#include "qemu/typedefs.h"
>  
>  #define MAX_RING_NAME_SZ 32
>  
> diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
> index e306fba..f85c7b2 100644
> --- a/hw/rdma/rdma_backend.c
> +++ b/hw/rdma/rdma_backend.c
> @@ -13,9 +13,9 @@
>   *
>   */
>  
> -#include <qemu/osdep.h>
> -#include <qemu/error-report.h>
> -#include <qapi/error.h>
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "qapi/error.h"
>  
>  #include <infiniband/verbs.h>
>  
> diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
> index b5fc45d..80ffc94 100644
> --- a/hw/rdma/rdma_rm.c
> +++ b/hw/rdma/rdma_rm.c
> @@ -13,9 +13,9 @@
>   *
>   */
>  
> -#include <qemu/osdep.h>
> -#include <qapi/error.h>
> -#include <cpu.h>
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "cpu.h"
>  
>  #include "rdma_utils.h"
>  #include "rdma_backend.h"
> diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
> index 293dfed..a9fa1ef 100644
> --- a/hw/rdma/vmw/pvrdma_cmd.c
> +++ b/hw/rdma/vmw/pvrdma_cmd.c
> @@ -13,9 +13,9 @@
>   *
>   */
>  
> -#include <qemu/osdep.h>
> -#include <qemu/error-report.h>
> -#include <cpu.h>
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "cpu.h"
>  #include <linux/types.h>
>  #include "hw/hw.h"
>  #include "hw/pci/pci.h"
> @@ -26,7 +26,7 @@
>  #include "../rdma_utils.h"
>  
>  #include "pvrdma.h"
> -#include <standard-headers/rdma/vmw_pvrdma-abi.h>
> +#include "standard-headers/rdma/vmw_pvrdma-abi.h"
>  
>  static void *pvrdma_map_to_pdir(PCIDevice *pdev, uint64_t pdir_dma,
>                                  uint32_t nchunks, size_t length)
> diff --git a/hw/rdma/vmw/pvrdma_dev_ring.c b/hw/rdma/vmw/pvrdma_dev_ring.c
> index ec309da..ff19a9e 100644
> --- a/hw/rdma/vmw/pvrdma_dev_ring.c
> +++ b/hw/rdma/vmw/pvrdma_dev_ring.c
> @@ -13,12 +13,12 @@
>   *
>   */
>  
> -#include <qemu/osdep.h>
> -#include <hw/pci/pci.h>
> -#include <cpu.h>
> +#include "qemu/osdep.h"
> +#include "hw/pci/pci.h"
> +#include "cpu.h"
>  
>  #include "../rdma_utils.h"
> -#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h>
> +#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h"
>  #include "pvrdma_dev_ring.h"
>  
>  int pvrdma_ring_init(PvrdmaRing *ring, const char *name, PCIDevice *dev,
> diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> index 9978781..25ea02a 100644
> --- a/hw/rdma/vmw/pvrdma_main.c
> +++ b/hw/rdma/vmw/pvrdma_main.c
> @@ -13,16 +13,16 @@
>   *
>   */
>  
> -#include <qemu/osdep.h>
> -#include <qapi/error.h>
> -#include <hw/hw.h>
> -#include <hw/pci/pci.h>
> -#include <hw/pci/pci_ids.h>
> -#include <hw/pci/msi.h>
> -#include <hw/pci/msix.h>
> -#include <hw/qdev-core.h>
> -#include <hw/qdev-properties.h>
> -#include <cpu.h>
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/hw.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pci_ids.h"
> +#include "hw/pci/msi.h"
> +#include "hw/pci/msix.h"
> +#include "hw/qdev-core.h"
> +#include "hw/qdev-properties.h"
> +#include "cpu.h"
>  #include "trace.h"
>  
>  #include "../rdma_rm.h"
> @@ -31,8 +31,8 @@
>  
>  #include <infiniband/verbs.h>
>  #include "pvrdma.h"
> -#include <standard-headers/rdma/vmw_pvrdma-abi.h>
> -#include <standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h>
> +#include "standard-headers/rdma/vmw_pvrdma-abi.h"
> +#include "standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_dev_api.h"
>  #include "pvrdma_qp_ops.h"
>  
>  static Property pvrdma_dev_properties[] = {
> diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c
> index f0a1f9e..a693c06 100644
> --- a/hw/rdma/vmw/pvrdma_qp_ops.c
> +++ b/hw/rdma/vmw/pvrdma_qp_ops.c
> @@ -13,14 +13,14 @@
>   *
>   */
>  
> -#include <qemu/osdep.h>
> +#include "qemu/osdep.h"
>  
>  #include "../rdma_utils.h"
>  #include "../rdma_rm.h"
>  #include "../rdma_backend.h"
>  
>  #include "pvrdma.h"
> -#include <standard-headers/rdma/vmw_pvrdma-abi.h>
> +#include "standard-headers/rdma/vmw_pvrdma-abi.h"
>  #include "pvrdma_qp_ops.h"
>  
>  typedef struct CompHandlerCtx {
> -- 
> MST
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote
  2018-03-21 15:22 ` [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote Michael S. Tsirkin
  2018-03-21 15:36   ` Daniel P. Berrangé
@ 2018-03-22 21:20   ` Stefan Hajnoczi
  1 sibling, 0 replies; 10+ messages in thread
From: Stefan Hajnoczi @ 2018-03-22 21:20 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: qemu-devel, Yuval Shaia, Marcel Apfelbaum, Paolo Bonzini,
	Yang Zhong, Philippe Mathieu-Daudé,
	Richard Henderson, Peter Maydell, Gerd Hoffmann, Fam Zheng,
	Marc-André Lureau, Daniel P. Berrangé,
	Stefan Weil

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

On Wed, Mar 21, 2018 at 05:22:07PM +0200, Michael S. Tsirkin wrote:
> Our rule right now is to use <> for external headers,
> "" for internal ones. The idea was to avoid conflicts
> between e.g. a system file named <trace.h> and an
> internal one by the same name.
> 
> Unfortunately we use -I compiler flag so it does not
> help: a system file doing #include <trace.h> will
> still pick up ours first.
> 
> To fix, switch to -iquote which is supported by both
> gcc and clang and only affects #include "" directives.
> 
> As a side effect, this catches any future uses of
>  #include <> for internal headers.
> 
> Suggested-by: Stefan Weil <sw@weilnetz.de>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> I still think we want to switch to a more formal rule such as qemu/
> prefix for all includes down the road, but this will at least catch any
> scheme violations from creeping in meanwhile.
> 
> 
>  configure       | 16 ++++++++--------
>  rules.mak       |  2 +-
>  Makefile.target |  4 ++--
>  3 files changed, 11 insertions(+), 11 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-03-22 21:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-21 15:22 [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives Michael S. Tsirkin
2018-03-21 15:22 ` [Qemu-devel] [PATCH for-2.12 2/2] make: switch from -I to -iquote Michael S. Tsirkin
2018-03-21 15:36   ` Daniel P. Berrangé
2018-03-21 16:02     ` Michael S. Tsirkin
2018-03-21 16:40       ` Marcel Apfelbaum
2018-03-21 19:13         ` Michael S. Tsirkin
2018-03-22 21:20   ` Stefan Hajnoczi
2018-03-21 15:38 ` [Qemu-devel] [PATCH for-2.12 1/2] rdma: fix up include directives Daniel P. Berrangé
2018-03-21 16:35 ` Marcel Apfelbaum
2018-03-21 21:15 ` Yuval Shaia

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.