[RFC] UAPI: Check headers by compiling all together as C++

[RFC] UAPI: Check headers by compiling all together as C++

[David Howells]

Here's a set of patches that inserts a step into the build process to make
sure that the UAPI headers can all be built together with C++ (if the
compiler being used supports C++).  All but the final patch perform fixups,
including:

 (1) Fix member names that conflict with C++ reserved words by providing
     alternates that can be used anywhere.  An anonymous union is used so
     that that the conflicting name is still available outside of C++.

 (2) Fix the use of flexible arrays in structs that get embedded (which is
     illegal in C++).

 (3) Remove the use of internal kernel structs in UAPI structures.

 (4) Fix symbol collisions.

 (5) Replace usage of u32 and co. with __u32 and co.

 (6) Fix use of sparsely initialised arrays (which g++ doesn't implement).

 (7) Remove some use of PAGE_SIZE since this isn't valid outside of the
     kernel.

And lastly:

 (8) Compile all of the UAPI headers (with a few exceptions) together as
     C++ to catch new errors occurring as part of the regular build
     process.

The patches can also be found here:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=uapi-check

Thanks,
David
---
David Howells (11):
      UAPI: drm: Fix use of C++ keywords as structural members
      UAPI: keys: Fix use of C++ keywords as structural members
      UAPI: virtio_net: Fix use of C++ keywords as structural members
      UAPI: bcache: Fix use of embedded flexible array
      UAPI: coda: Don't use internal kernel structs in UAPI
      UAPI: netfilter: Fix symbol collision issues
      UAPI: nilfs2: Fix use of undefined byteswapping functions
      UAPI: sound: Fix use of u32 and co. in UAPI headers
      UAPI: ndctl: Fix g++-unsupported initialisation in headers
      UAPI: ndctl: Remove use of PAGE_SIZE
      UAPI: Check headers build for C++


 Makefile                                          |    1 
 include/linux/ndctl.h                             |   22 ++++
 include/uapi/drm/i810_drm.h                       |    7 +
 include/uapi/drm/msm_drm.h                        |    7 +
 include/uapi/linux/bcache.h                       |    2 
 include/uapi/linux/coda_psdev.h                   |    4 +
 include/uapi/linux/keyctl.h                       |    7 +
 include/uapi/linux/ndctl.h                        |   20 ++-
 include/uapi/linux/netfilter/nfnetlink_cthelper.h |    2 
 include/uapi/linux/netfilter_ipv4/ipt_ECN.h       |    9 --
 include/uapi/linux/nilfs2_ondisk.h                |   21 ++--
 include/uapi/linux/virtio_net.h                   |    7 +
 include/uapi/sound/skl-tplg-interface.h           |  106 +++++++++---------
 scripts/headers-c++.sh                            |  124 +++++++++++++++++++++
 14 files changed, 255 insertions(+), 84 deletions(-)
 create mode 100644 include/linux/ndctl.h
 create mode 100755 scripts/headers-c++.sh

[PATCH 01/11] UAPI: drm: Fix use of C++ keywords as structural members

[David Howells]

The i810 and msm drm drivers use C++ keywords as structural members.  Fix
this by inserting an anonymous union that provides an alternative name and
then hide the reserved name in C++.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Rob Clark <robdclark@gmail.com>
cc: David Airlie <airlied@linux.ie>
cc: linux-arm-msm@vger.kernel.org
cc: dri-devel@lists.freedesktop.org
cc: freedreno@lists.freedesktop.org
---

 include/uapi/drm/i810_drm.h |    7 ++++++-
 include/uapi/drm/msm_drm.h  |    7 ++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/uapi/drm/i810_drm.h b/include/uapi/drm/i810_drm.h
index d285d5e72e6a..617d79ec3fc5 100644
--- a/include/uapi/drm/i810_drm.h
+++ b/include/uapi/drm/i810_drm.h
@@ -266,7 +266,12 @@ typedef struct _drm_i810_copy_t {
 #define PR_MASK              (0x7<<18)
 
 typedef struct drm_i810_dma {
-	void *virtual;
+	union {
+#ifndef __cplusplus
+		void *virtual;
+#endif
+		void *_virtual;
+	};
 	int request_idx;
 	int request_size;
 	int granted;
diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
index c06d0a5bdd80..e99bab72d58c 100644
--- a/include/uapi/drm/msm_drm.h
+++ b/include/uapi/drm/msm_drm.h
@@ -148,7 +148,12 @@ struct drm_msm_gem_cpu_fini {
  */
 struct drm_msm_gem_submit_reloc {
 	__u32 submit_offset;  /* in, offset from submit_bo */
-	__u32 or;             /* in, value OR'd with result */
+	union {
+#ifndef __cplusplus
+		__u32 or;	/* in, value OR'd with result */
+#endif
+		__u32 _or;	/* in, value OR'd with result */
+	};
 	__s32 shift;          /* in, amount of left shift (can be negative) */
 	__u32 reloc_idx;      /* in, index of reloc_bo buffer */
 	__u64 reloc_offset;   /* in, offset from start of reloc_bo */

[PATCH 02/11] UAPI: keys: Fix use of C++ keywords as structural members

[David Howells]

The keyctl_dh_params struct uses a C++ keyword as structural members.  Fix
this by inserting an anonymous union that provides an alternative name and
then hide the reserved name in C++.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Mat Martineau <mathew.j.martineau@linux.intel.com>
cc: keyrings@vger.kernel.org
---

 include/uapi/linux/keyctl.h |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
index 7b8c9e19bad1..170f015d1f25 100644
--- a/include/uapi/linux/keyctl.h
+++ b/include/uapi/linux/keyctl.h
@@ -65,7 +65,12 @@
 
 /* keyctl structures */
 struct keyctl_dh_params {
-	__s32 private;
+	union {
+#ifndef __cplusplus
+		__s32 private;
+#endif
+		__s32 dh_private;
+	};
 	__s32 prime;
 	__s32 base;
 };

[PATCH 03/11] UAPI: virtio_net: Fix use of C++ keywords as structural members

[David Howells]

The virtio_net_ctrl_hdr struct uses a C++ keyword as structural members.  Fix
this by inserting an anonymous union that provides an alternative name and
then hide the reserved name in C++.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: "Michael S. Tsirkin" <mst@redhat.com>
cc: Jason Wang <jasowang@redhat.com>
cc: virtualization@lists.linux-foundation.org
---

 include/uapi/linux/virtio_net.h |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index a3715a3224c1..967142bc0e05 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -150,7 +150,12 @@ struct virtio_net_hdr_mrg_rxbuf {
  * command goes in between.
  */
 struct virtio_net_ctrl_hdr {
-	__u8 class;
+	union {
+#ifndef __cplusplus
+		__u8 class;
+#endif
+		__u8 _class;
+	};
 	__u8 cmd;
 } __attribute__((packed));
 

[PATCH 04/11] UAPI: bcache: Fix use of embedded flexible array

[David Howells]

The bkey struct defined by bcache is embedded in the jset struct.  However,
this is illegal in C++ as there's a "flexible array" at the end of the
struct.  Change this to be a 0-length struct instead.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Coly Li <colyli@suse.de>
cc: Kent Overstreet <kent.overstreet@gmail.com>
cc: linux-bcache@vger.kernel.org
---

 include/uapi/linux/bcache.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/bcache.h b/include/uapi/linux/bcache.h
index 5d4f58e059fd..11863e903bff 100644
--- a/include/uapi/linux/bcache.h
+++ b/include/uapi/linux/bcache.h
@@ -23,7 +23,7 @@ static inline void SET_##name(type *k, __u64 v)			\
 struct bkey {
 	__u64	high;
 	__u64	low;
-	__u64	ptr[];
+	__u64	ptr[0];
 };
 
 #define KEY_FIELD(name, field, offset, size)				\

[PATCH 05/11] UAPI: coda: Don't use internal kernel structs in UAPI

[David Howells]

The size and layout of internal kernel structures may not be relied upon
outside of the kernel and may even change in a containerised environment if
a container image is frozen and shifted to another machine.

Excise these from Coda's upc_req struct.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jan Harkes <jaharkes@cs.cmu.edu>
cc: coda@cs.cmu.edu
cc: codalist@coda.cs.cmu.edu
cc: linux-fsdevel@vger.kernel.org
---

 include/uapi/linux/coda_psdev.h |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/uapi/linux/coda_psdev.h b/include/uapi/linux/coda_psdev.h
index aa6623efd2dd..9c3acde393cd 100644
--- a/include/uapi/linux/coda_psdev.h
+++ b/include/uapi/linux/coda_psdev.h
@@ -10,14 +10,18 @@
 
 /* messages between coda filesystem in kernel and Venus */
 struct upc_req {
+#ifdef __KERNEL__
 	struct list_head    uc_chain;
+#endif
 	caddr_t	            uc_data;
 	u_short	            uc_flags;
 	u_short             uc_inSize;  /* Size is at most 5000 bytes */
 	u_short	            uc_outSize;
 	u_short	            uc_opcode;  /* copied from data to save lookup */
 	int		    uc_unique;
+#ifdef __KERNEL__
 	wait_queue_head_t   uc_sleep;   /* process' wait queue */
+#endif
 };
 
 #define CODA_REQ_ASYNC  0x1

[PATCH 06/11] UAPI: netfilter: Fix symbol collision issues

[David Howells]

The netfilter UAPI headers have some symbol collision issues:

 (1) "enum nfnl_acct_msg_types" is defined twice, and each definition is
     completely different.

     Fix this by renaming the one in nfnetlink_cthelper.h to be "enum
     nfnl_cthelper_types" to be consistent with the other things in that
     file.

 (2) There's a disagreement between ipt_ECN.h and ipt_ecn.h over the
     definition of various IPT_ECN_* constants, leading to an error over
     IPT_ECN_IP_MASK being substituted when being defined as an enum value
     in ipt_ecn.h if ipt_ECN.h is #included first.

     Fix this by removing the conflicting constants from ipt_ECN.h and
     including ipt_ecn.h instead.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: netfilter-devel@vger.kernel.org
cc: coreteam@netfilter.org
---

 include/uapi/linux/netfilter/nfnetlink_cthelper.h |    2 +-
 include/uapi/linux/netfilter_ipv4/ipt_ECN.h       |    9 +--------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/include/uapi/linux/netfilter/nfnetlink_cthelper.h b/include/uapi/linux/netfilter/nfnetlink_cthelper.h
index a13137afc429..b9313ed0c313 100644
--- a/include/uapi/linux/netfilter/nfnetlink_cthelper.h
+++ b/include/uapi/linux/netfilter/nfnetlink_cthelper.h
@@ -5,7 +5,7 @@
 #define NFCT_HELPER_STATUS_DISABLED	0
 #define NFCT_HELPER_STATUS_ENABLED	1
 
-enum nfnl_acct_msg_types {
+enum nfnl_cthelper_types {
 	NFNL_MSG_CTHELPER_NEW,
 	NFNL_MSG_CTHELPER_GET,
 	NFNL_MSG_CTHELPER_DEL,
diff --git a/include/uapi/linux/netfilter_ipv4/ipt_ECN.h b/include/uapi/linux/netfilter_ipv4/ipt_ECN.h
index e3630fd045b8..d582119ad62a 100644
--- a/include/uapi/linux/netfilter_ipv4/ipt_ECN.h
+++ b/include/uapi/linux/netfilter_ipv4/ipt_ECN.h
@@ -12,14 +12,7 @@
 
 #include <linux/types.h>
 #include <linux/netfilter/xt_DSCP.h>
-
-#define IPT_ECN_IP_MASK	(~XT_DSCP_MASK)
-
-#define IPT_ECN_OP_SET_IP	0x01	/* set ECN bits of IPv4 header */
-#define IPT_ECN_OP_SET_ECE	0x10	/* set ECE bit of TCP header */
-#define IPT_ECN_OP_SET_CWR	0x20	/* set CWR bit of TCP header */
-
-#define IPT_ECN_OP_MASK		0xce
+#include <linux/netfilter_ipv4/ipt_ecn.h>
 
 struct ipt_ECN_info {
 	__u8 operation;	/* bitset of operations */

[PATCH 07/11] UAPI: nilfs2: Fix use of undefined byteswapping functions

[David Howells]

nilfs2 exports a load of inline functions to userspace that call kernel
byteswapping functions that don't exist in UAPI.  Fix this by making it
#include asm/byteorder.h and use the functions declared there.

A better way is probably to remove these inline functions from the nilfs2
header since they are technically broken.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Ryusuke Konishi <konishi.ryusuke@lab.ntt.co.jp>
cc: linux-nilfs@vger.kernel.org
cc: linux-fsdevel@vger.kernel.org
---

 include/uapi/linux/nilfs2_ondisk.h |   21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/include/uapi/linux/nilfs2_ondisk.h b/include/uapi/linux/nilfs2_ondisk.h
index a7e66ab11d1d..d3bd2fe08791 100644
--- a/include/uapi/linux/nilfs2_ondisk.h
+++ b/include/uapi/linux/nilfs2_ondisk.h
@@ -29,6 +29,7 @@
 
 #include <linux/types.h>
 #include <linux/magic.h>
+#include <asm/byteorder.h>
 
 
 #define NILFS_INODE_BMAP_SIZE	7
@@ -533,19 +534,19 @@ enum {
 static inline void							\
 nilfs_checkpoint_set_##name(struct nilfs_checkpoint *cp)		\
 {									\
-	cp->cp_flags = cpu_to_le32(le32_to_cpu(cp->cp_flags) |		\
+	cp->cp_flags = __cpu_to_le32(__le32_to_cpu(cp->cp_flags) |	\
 				   (1UL << NILFS_CHECKPOINT_##flag));	\
 }									\
 static inline void							\
 nilfs_checkpoint_clear_##name(struct nilfs_checkpoint *cp)		\
 {									\
-	cp->cp_flags = cpu_to_le32(le32_to_cpu(cp->cp_flags) &		\
+	cp->cp_flags = __cpu_to_le32(__le32_to_cpu(cp->cp_flags) &	\
 				   ~(1UL << NILFS_CHECKPOINT_##flag));	\
 }									\
 static inline int							\
 nilfs_checkpoint_##name(const struct nilfs_checkpoint *cp)		\
 {									\
-	return !!(le32_to_cpu(cp->cp_flags) &				\
+	return !!(__le32_to_cpu(cp->cp_flags) &				\
 		  (1UL << NILFS_CHECKPOINT_##flag));			\
 }
 
@@ -595,20 +596,20 @@ enum {
 static inline void							\
 nilfs_segment_usage_set_##name(struct nilfs_segment_usage *su)		\
 {									\
-	su->su_flags = cpu_to_le32(le32_to_cpu(su->su_flags) |		\
+	su->su_flags = __cpu_to_le32(__le32_to_cpu(su->su_flags) |	\
 				   (1UL << NILFS_SEGMENT_USAGE_##flag));\
 }									\
 static inline void							\
 nilfs_segment_usage_clear_##name(struct nilfs_segment_usage *su)	\
 {									\
 	su->su_flags =							\
-		cpu_to_le32(le32_to_cpu(su->su_flags) &			\
+		__cpu_to_le32(__le32_to_cpu(su->su_flags) &		\
 			    ~(1UL << NILFS_SEGMENT_USAGE_##flag));      \
 }									\
 static inline int							\
 nilfs_segment_usage_##name(const struct nilfs_segment_usage *su)	\
 {									\
-	return !!(le32_to_cpu(su->su_flags) &				\
+	return !!(__le32_to_cpu(su->su_flags) &				\
 		  (1UL << NILFS_SEGMENT_USAGE_##flag));			\
 }
 
@@ -619,15 +620,15 @@ NILFS_SEGMENT_USAGE_FNS(ERROR, error)
 static inline void
 nilfs_segment_usage_set_clean(struct nilfs_segment_usage *su)
 {
-	su->su_lastmod = cpu_to_le64(0);
-	su->su_nblocks = cpu_to_le32(0);
-	su->su_flags = cpu_to_le32(0);
+	su->su_lastmod = __cpu_to_le64(0);
+	su->su_nblocks = __cpu_to_le32(0);
+	su->su_flags = __cpu_to_le32(0);
 }
 
 static inline int
 nilfs_segment_usage_clean(const struct nilfs_segment_usage *su)
 {
-	return !le32_to_cpu(su->su_flags);
+	return !__le32_to_cpu(su->su_flags);
 }
 
 /**

[PATCH 08/11] UAPI: sound: Fix use of u32 and co. in UAPI headers

[David Howells]

Fix the use of u32 and co. in UAPI headers as these are not defined.  Switch
to using the __u32-style equivalents instead.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jaroslav Kysela <perex@perex.cz>
cc: Takashi Iwai <tiwai@suse.com>
cc: alsa-devel@alsa-project.org (moderated for non-subscribers)
---

 include/uapi/sound/skl-tplg-interface.h |  106 ++++++++++++++++---------------
 1 file changed, 54 insertions(+), 52 deletions(-)

diff --git a/include/uapi/sound/skl-tplg-interface.h b/include/uapi/sound/skl-tplg-interface.h
index f58cafa42f18..f39352cef382 100644
--- a/include/uapi/sound/skl-tplg-interface.h
+++ b/include/uapi/sound/skl-tplg-interface.h
@@ -10,6 +10,8 @@
 #ifndef __HDA_TPLG_INTERFACE_H__
 #define __HDA_TPLG_INTERFACE_H__
 
+#include <linux/types.h>
+
 /*
  * Default types range from 0~12. type can range from 0 to 0xff
  * SST types start at higher to avoid any overlapping in future
@@ -143,10 +145,10 @@ enum skl_module_param_type {
 };
 
 struct skl_dfw_algo_data {
-	u32 set_params:2;
-	u32 rsvd:30;
-	u32 param_id;
-	u32 max;
+	__u32 set_params:2;
+	__u32 rsvd:30;
+	__u32 param_id;
+	__u32 max;
 	char params[0];
 } __packed;
 
@@ -163,68 +165,68 @@ enum skl_tuple_type {
 /* v4 configuration data */
 
 struct skl_dfw_v4_module_pin {
-	u16 module_id;
-	u16 instance_id;
+	__u16 module_id;
+	__u16 instance_id;
 } __packed;
 
 struct skl_dfw_v4_module_fmt {
-	u32 channels;
-	u32 freq;
-	u32 bit_depth;
-	u32 valid_bit_depth;
-	u32 ch_cfg;
-	u32 interleaving_style;
-	u32 sample_type;
-	u32 ch_map;
+	__u32 channels;
+	__u32 freq;
+	__u32 bit_depth;
+	__u32 valid_bit_depth;
+	__u32 ch_cfg;
+	__u32 interleaving_style;
+	__u32 sample_type;
+	__u32 ch_map;
 } __packed;
 
 struct skl_dfw_v4_module_caps {
-	u32 set_params:2;
-	u32 rsvd:30;
-	u32 param_id;
-	u32 caps_size;
-	u32 caps[HDA_SST_CFG_MAX];
+	__u32 set_params:2;
+	__u32 rsvd:30;
+	__u32 param_id;
+	__u32 caps_size;
+	__u32 caps[HDA_SST_CFG_MAX];
 } __packed;
 
 struct skl_dfw_v4_pipe {
-	u8 pipe_id;
-	u8 pipe_priority;
-	u16 conn_type:4;
-	u16 rsvd:4;
-	u16 memory_pages:8;
+	__u8 pipe_id;
+	__u8 pipe_priority;
+	__u16 conn_type:4;
+	__u16 rsvd:4;
+	__u16 memory_pages:8;
 } __packed;
 
 struct skl_dfw_v4_module {
 	char uuid[SKL_UUID_STR_SZ];
 
-	u16 module_id;
-	u16 instance_id;
-	u32 max_mcps;
-	u32 mem_pages;
-	u32 obs;
-	u32 ibs;
-	u32 vbus_id;
-
-	u32 max_in_queue:8;
-	u32 max_out_queue:8;
-	u32 time_slot:8;
-	u32 core_id:4;
-	u32 rsvd1:4;
-
-	u32 module_type:8;
-	u32 conn_type:4;
-	u32 dev_type:4;
-	u32 hw_conn_type:4;
-	u32 rsvd2:12;
-
-	u32 params_fixup:8;
-	u32 converter:8;
-	u32 input_pin_type:1;
-	u32 output_pin_type:1;
-	u32 is_dynamic_in_pin:1;
-	u32 is_dynamic_out_pin:1;
-	u32 is_loadable:1;
-	u32 rsvd3:11;
+	__u16 module_id;
+	__u16 instance_id;
+	__u32 max_mcps;
+	__u32 mem_pages;
+	__u32 obs;
+	__u32 ibs;
+	__u32 vbus_id;
+
+	__u32 max_in_queue:8;
+	__u32 max_out_queue:8;
+	__u32 time_slot:8;
+	__u32 core_id:4;
+	__u32 rsvd1:4;
+
+	__u32 module_type:8;
+	__u32 conn_type:4;
+	__u32 dev_type:4;
+	__u32 hw_conn_type:4;
+	__u32 rsvd2:12;
+
+	__u32 params_fixup:8;
+	__u32 converter:8;
+	__u32 input_pin_type:1;
+	__u32 output_pin_type:1;
+	__u32 is_dynamic_in_pin:1;
+	__u32 is_dynamic_out_pin:1;
+	__u32 is_loadable:1;
+	__u32 rsvd3:11;
 
 	struct skl_dfw_v4_pipe pipe;
 	struct skl_dfw_v4_module_fmt in_fmt[MAX_IN_QUEUE];

[PATCH 09/11] UAPI: ndctl: Fix g++-unsupported initialisation in headers

[David Howells]

The following code in the linux/ndctl header file:

	static inline const char *nvdimm_bus_cmd_name(unsigned cmd)
	{
		static const char * const names[] = {
			[ND_CMD_ARS_CAP] = "ars_cap",
			[ND_CMD_ARS_START] = "ars_start",
			[ND_CMD_ARS_STATUS] = "ars_status",
			[ND_CMD_CLEAR_ERROR] = "clear_error",
			[ND_CMD_CALL] = "cmd_call",
		};

		if (cmd < ARRAY_SIZE(names) && names[cmd])
			return names[cmd];
		return "unknown";
	}

is broken in a number of ways:

 (1) ARRAY_SIZE() is not generally defined.  Fix this by defining a label
     in the enum that indicates the number of commands.

 (2) g++ does not support "non-trivial" array initialisers fully yet.  Fix
     this by defining the missing intermediate values.

 (3) Every file that calls this function will acquire a copy of names[].

The same goes for nvdimm_cmd_name().

A better way would be to remove these functions and their arrays from the
header entirely.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Dan Williams <dan.j.williams@intel.com>
cc: linux-nvdimm@lists.01.org
---

 include/uapi/linux/ndctl.h |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
index 7e27070b9440..9c89159f6a0f 100644
--- a/include/uapi/linux/ndctl.h
+++ b/include/uapi/linux/ndctl.h
@@ -117,6 +117,7 @@ enum {
 	ND_CMD_VENDOR_EFFECT_LOG = 8,
 	ND_CMD_VENDOR = 9,
 	ND_CMD_CALL = 10,
+	nr__ND_CMD = 11
 };
 
 enum {
@@ -128,22 +129,29 @@ enum {
 
 static inline const char *nvdimm_bus_cmd_name(unsigned cmd)
 {
-	static const char * const names[] = {
+	static const char * const names[nr__ND_CMD] = {
+		[0] = NULL,
 		[ND_CMD_ARS_CAP] = "ars_cap",
 		[ND_CMD_ARS_START] = "ars_start",
 		[ND_CMD_ARS_STATUS] = "ars_status",
 		[ND_CMD_CLEAR_ERROR] = "clear_error",
+		[5] = NULL,
+		[6] = NULL,
+		[7] = NULL,
+		[8] = NULL,
+		[9] = NULL,
 		[ND_CMD_CALL] = "cmd_call",
 	};
 
-	if (cmd < ARRAY_SIZE(names) && names[cmd])
+	if (cmd < nr__ND_CMD && names[cmd])
 		return names[cmd];
 	return "unknown";
 }
 
 static inline const char *nvdimm_cmd_name(unsigned cmd)
 {
-	static const char * const names[] = {
+	static const char * const names[nr__ND_CMD] = {
+		[0] = NULL,
 		[ND_CMD_SMART] = "smart",
 		[ND_CMD_SMART_THRESHOLD] = "smart_thresh",
 		[ND_CMD_DIMM_FLAGS] = "flags",
@@ -156,7 +164,7 @@ static inline const char *nvdimm_cmd_name(unsigned cmd)
 		[ND_CMD_CALL] = "cmd_call",
 	};
 
-	if (cmd < ARRAY_SIZE(names) && names[cmd])
+	if (cmd < nr__ND_CMD && names[cmd])
 		return names[cmd];
 	return "unknown";
 }

[PATCH 10/11] UAPI: ndctl: Remove use of PAGE_SIZE

[David Howells]

The macro PAGE_SIZE isn't valid outside of the kernel, so it should not
appear in UAPI headers.

Furthermore, the actual machine page size could theoretically change from
an application's point of view if it's running in a container that gets
migrated to another machine (say 4K/ppc64 to 64K/ppc64).

Fixes: f2ba5a5baecf ("libnvdimm, namespace: make min namespace size 4K")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Dan Williams <dan.j.williams@intel.com>
cc: linux-nvdimm@lists.01.org
---

 include/linux/ndctl.h      |   22 ++++++++++++++++++++++
 include/uapi/linux/ndctl.h |    4 ----
 2 files changed, 22 insertions(+), 4 deletions(-)
 create mode 100644 include/linux/ndctl.h

diff --git a/include/linux/ndctl.h b/include/linux/ndctl.h
new file mode 100644
index 000000000000..cd5a293ce3ae
--- /dev/null
+++ b/include/linux/ndctl.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2014-2016, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU Lesser General Public License,
+ * version 2.1, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for
+ * more details.
+ */
+#ifndef _LINUX_NDCTL_H
+#define _LINUX_NDCTL_H
+
+#include <uapi/linux/ndctl.h>
+
+enum {
+	ND_MIN_NAMESPACE_SIZE = PAGE_SIZE,
+};
+
+#endif /* _LINUX_NDCTL_H */
diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
index 9c89159f6a0f..bcda968e6d80 100644
--- a/include/uapi/linux/ndctl.h
+++ b/include/uapi/linux/ndctl.h
@@ -216,10 +216,6 @@ enum nd_driver_flags {
 	ND_DRIVER_DAX_PMEM	  = 1 << ND_DEVICE_DAX_PMEM,
 };
 
-enum {
-	ND_MIN_NAMESPACE_SIZE = PAGE_SIZE,
-};
-
 enum ars_masks {
 	ARS_STATUS_MASK = 0x0000FFFF,
 	ARS_EXT_STATUS_SHIFT = 16,

[PATCH 11/11] UAPI: Check headers build for C++

[David Howells]

Check that all the headers can be included from one file and built for C++,
thereby catching the use of C++ reserved words and bits of unimplemented
C++ in the UAPI headers.

Note that certain headers are excluded from the build, including:

 (1) Any header ending in "_32.h", "_64.h" or "_x32.h" as these are
     expected to be multiarch variant headers.

 (2) Endianness variant headers.

 (3) asm-generic/ headers (they're used conditionally by the asm/ headers
     and shouldn't be used directly).

 (4) netfilter_ipv*/ip*t_LOG.h headers.  They emit a warning indicating
     they're going to be removed soon.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Masahiro Yamada <yamada.masahiro@socionext.com>
cc: Michal Marek <michal.lkml@markovi.net>
cc: linux-kbuild@vger.kernel.org
---

 Makefile               |    1 
 scripts/headers-c++.sh |  124 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 125 insertions(+)
 create mode 100755 scripts/headers-c++.sh

diff --git a/Makefile b/Makefile
index 2b458801ba74..f3c36c2bb4cf 100644
--- a/Makefile
+++ b/Makefile
@@ -1183,6 +1183,7 @@ headers_install: __headers
 	  $(error Headers not exportable for the $(SRCARCH) architecture))
 	$(Q)$(MAKE) $(hdr-inst)=include/uapi dst=include
 	$(Q)$(MAKE) $(hdr-inst)=arch/$(SRCARCH)/include/uapi $(hdr-dst)
+	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/headers-c++.sh check
 
 PHONY += headers_check_all
 headers_check_all: headers_install_all
diff --git a/scripts/headers-c++.sh b/scripts/headers-c++.sh
new file mode 100755
index 000000000000..7e56913629f8
--- /dev/null
+++ b/scripts/headers-c++.sh
@@ -0,0 +1,124 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Run headers_$1 command for all suitable architectures
+
+# Stop on error
+set -e
+
+if ! $CC -x c++ -c - -o /dev/null </dev/null 2>/dev/null
+then
+    echo "  CHECK   C++ HEADER COMPILATION [SKIPPED]"
+    exit 0
+fi
+
+echo "  CHECK   C++ HEADER COMPILATION"
+
+mkdir -p hdr-check
+cd hdr-check
+
+mkdir -p include/sys
+mkdir -p include/arpa
+mkdir -p include/xen/interface
+echo >include/endian.h
+echo >include/limits.h
+echo >include/stdint.h
+echo >include/stdlib.h
+echo >include/stdio.h
+echo >include/string.h
+echo >include/time.h
+echo >include/unistd.h
+echo >include/arpa/inet.h
+echo >include/sys/ioctl.h
+echo >include/sys/types.h
+echo >include/sys/time.h
+echo >include/sys/socket.h
+echo >include/xen/interface/xen.h
+
+cat >test.h <<EOF
+#ifdef __cplusplus
+#define NULL nullptr
+#define _Bool bool
+#else
+#define NULL ((void *)0)
+#define bool _Bool
+#endif
+#include <linux/types.h>
+#include <linux/socket.h>
+#include <linux/time.h>
+
+typedef __s8			int8_t;
+typedef __s16			int16_t;
+typedef __s32			int32_t;
+typedef __s64			int64_t;
+typedef __u8			uint8_t;
+typedef __u16			uint16_t;
+typedef __u32			uint32_t;
+typedef __u64			uint64_t;
+typedef long int		intptr_t;
+typedef unsigned long int	uintptr_t;
+typedef unsigned short		u_short;
+typedef unsigned int		u_int;
+typedef unsigned long		u_long;
+typedef char			*caddr_t;
+
+typedef __kernel_clockid_t	clockid_t;
+typedef __kernel_ino_t		ino_t;
+typedef __kernel_pid_t		pid_t;
+typedef __kernel_sa_family_t	sa_family_t;
+typedef __kernel_size_t		size_t;
+typedef __kernel_uid_t		uid_t;
+
+typedef unsigned long		elf_greg_t;
+typedef elf_greg_t		elf_gregset_t[1];
+typedef unsigned long long	elf_fpregset_t[1];
+typedef unsigned long long	elf_fpxregset_t[1];
+
+#define INT_MIN ((int)0x80000000)
+#define INT_MAX ((int)0x7fffffff)
+
+extern size_t strlen(const char *);
+extern void *memset(void *, int, size_t);
+extern void *memcpy(void *, const void *, size_t);
+extern __u16 ntohs(__u16);
+extern __u16 htons(__u16);
+extern __u32 ntohl(__u32);
+extern __u32 htonl(__u32);
+
+typedef uint32_t		grant_ref_t;
+typedef uint16_t		domid_t;
+typedef unsigned long		xen_pfn_t;
+
+#define MSG_FIN         0x200
+
+typedef int SVGA3dMSPattern;
+typedef int SVGA3dMSQualityLevel;
+
+struct sockaddr
+{
+	sa_family_t	sa_family;
+	char		sa_data[14];
+};
+#define sockaddr_storage __kernel_sockaddr_storage
+
+#define _LINUX_PATCHKEY_H_INDIRECT
+
+EOF
+
+find ../usr/include -name '*.h' |
+    grep -v 'linux/byteorder/big_endian.h' |
+    grep -v 'linux/byteorder/little_endian.h' |
+    grep -v '_\(32\|64\|x32\)[.]h$' |
+    grep -v '/asm-generic/' |
+    # ip*t_LOG.h are deprecated
+    grep -v 'linux/netfilter_ipv4/ipt_LOG[.]h' |
+    grep -v 'linux/netfilter_ipv6/ip6t_LOG[.]h' |
+    sed -e 's!../usr/include/!#include <!' -e 's!$!>!' >>test.h
+
+echo '#include "test.h"' >test.cpp
+
+$CC -x c++ -o /dev/null -c test.cpp \
+    -nostdinc \
+    -isystem ./include \
+    -isystem ../usr/include \
+    -fpermissive \
+    -D PAGE_SIZE='#PAGE_SIZE_IS_NOT_VALID_OUTSIDE_OF_KERNEL'

Re: [PATCH 03/11] UAPI: virtio_net: Fix use of C++ keywords as structural members

[Greg KH]

On Wed, Sep 05, 2018 at 04:54:55PM +0100, David Howells wrote:
> The virtio_net_ctrl_hdr struct uses a C++ keyword as structural members.  Fix
> this by inserting an anonymous union that provides an alternative name and
> then hide the reserved name in C++.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: "Michael S. Tsirkin" <mst@redhat.com>
> cc: Jason Wang <jasowang@redhat.com>
> cc: virtualization@lists.linux-foundation.org
> ---
> 
>  include/uapi/linux/virtio_net.h |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index a3715a3224c1..967142bc0e05 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -150,7 +150,12 @@ struct virtio_net_hdr_mrg_rxbuf {
>   * command goes in between.
>   */
>  struct virtio_net_ctrl_hdr {
> -	__u8 class;
> +	union {
> +#ifndef __cplusplus
> +		__u8 class;
> +#endif
> +		__u8 _class;
> +	};

Ugh, ick, no!

Come on now, either put the whole C namespace stuff around the file, or
don't care about this at all.  Doing this whack-a-mole style is a mess.

"class" is a fine variable name for C code, there's no reason this has
to change here at all.

greg k-h

Re: [PATCH 05/11] UAPI: coda: Don't use internal kernel structs in UAPI

[Jan Harkes]

On Wed, Sep 05, 2018 at 04:55:10PM +0100, David Howells wrote:
> The size and layout of internal kernel structures may not be relied upon
> outside of the kernel and may even change in a containerised environment if
> a container image is frozen and shifted to another machine.
> 
> Excise these from Coda's upc_req struct.

Argh, that won't work.

I still have to look at where this structure is used exactly, but...

Either this structure is used by the messages that the kernel sends to
userspace, in which case we don't want the kernel to pack the larger
structure that includes a list_head and a wait_queue_head_t in the
message while userspace reads as if it was a smaller structure without
those.

But my gut feeling is that this is not part of the upcall request
messages and never gets to userspace and as such shouldn't be in uapi to
begin with.

Jan

Re: [RFC] UAPI: Check headers by compiling all together as C++

[Greg KH]

On Wed, Sep 05, 2018 at 04:54:27PM +0100, David Howells wrote:
> 
> Here's a set of patches that inserts a step into the build process to make
> sure that the UAPI headers can all be built together with C++ (if the
> compiler being used supports C++).  All but the final patch perform fixups,
> including:

Wait, why do we care?  What has recently changed to start to directly
import kernel uapi files into C++ code?

And if userspace wants to do this, can't they do the C namespace trick
themselves when they do the import?  That must be how they are doing it
today, right?

thanks,

greg k-h

Re: [PATCH 05/11] UAPI: coda: Don't use internal kernel structs in UAPI

[Yann Droneaud]

Le mercredi 05 septembre 2018 à 16:55 +0100, David Howells a écrit :
> The size and layout of internal kernel structures may not be relied
> upon outside of the kernel and may even change in a containerised
> environment if a container image is frozen and shifted to another
> machine.
> 
> Excise these from Coda's upc_req struct.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Jan Harkes <jaharkes@cs.cmu.edu>
> cc: coda@cs.cmu.edu
> cc: codalist@coda.cs.cmu.edu
> cc: linux-fsdevel@vger.kernel.org
> ---
> 
>  include/uapi/linux/coda_psdev.h |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/uapi/linux/coda_psdev.h b/include/uapi/linux/coda_psdev.h
> index aa6623efd2dd..9c3acde393cd 100644
> --- a/include/uapi/linux/coda_psdev.h
> +++ b/include/uapi/linux/coda_psdev.h
> @@ -10,14 +10,18 @@
>  
>  /* messages between coda filesystem in kernel and Venus */
>  struct upc_req {
> +#ifdef __KERNEL__
>  	struct list_head    uc_chain;
> +#endif
>  	caddr_t	            uc_data;
>  	u_short	            uc_flags;
>  	u_short             uc_inSize;  /* Size is at most 5000 bytes */
>  	u_short	            uc_outSize;
>  	u_short	            uc_opcode;  /* copied from data to save lookup */
>  	int		    uc_unique;
> +#ifdef __KERNEL__
>  	wait_queue_head_t   uc_sleep;   /* process' wait queue */
> +#endif
>  };
>  

This structure should not have been exposed to userspace in the first
place: it's unusable by userspace as it is. It was incorrect to have it
outside of #ifdef __KERNEL__ before commit 607ca46e97a1b ... 

... and it's not exchanged between kernel and userspace, see
coda_psdev_write():

        struct upc_req *req = NULL;

        ...

        if (copy_from_user(req->uc_data, buf, nbytes)) {
                req->uc_flags |= CODA_REQ_ABORT;
                wake_up(&req->uc_sleep);
                retval = -EFAULT;
                goto out;
        }

Only data, a caddr_t, is read from userspace.

So the structure can be moved back to <linux/coda_psdev.h>.

>  #define CODA_REQ_ASYNC  0x1
> 

All CODA_REQ_* defines internals to kernel side and not exchanged with userspace.

Please move them back to <linux/coda_psdev.h>

Regards.

-- 
Yann Droneaud
OPTEYA

Re: [PATCH 03/11] UAPI: virtio_net: Fix use of C++ keywords as structural members

[David Howells]

Greg KH <gregkh@linuxfoundation.org> wrote:

> Come on now, either put the whole C namespace stuff around the file,

You mean wrap it with 'extern "C" { ... }'?  That doesn't fix it.  That only
affects the symbols generated by the compiler.

> "class" is a fine variable name for C code, there's no reason this has
> to change here at all.

I'm trying to prevent future accidents like the one in linux/keyctl.h.  The
easiest way to do this[**] is to pass the entire set of UAPI headers[*]
through the compiler together.

Besides I still have my dark plan to C++-ise the kernel[***] :-D

David

[*] with some obvious exceptions

[**] and it catches other errors too

[***] https://lkml.org/lkml/2018/4/1/116

Re: [PATCH 05/11] UAPI: coda: Don't use internal kernel structs in UAPI

[David Howells]

Yann Droneaud <ydroneaud@opteya.com> wrote:

> Please move them back to <linux/coda_psdev.h>

Will do.

David

Re: [PATCH 05/11] UAPI: coda: Don't use internal kernel structs in UAPI

[Jan Harkes]

On Wed, Sep 05, 2018 at 07:12:37PM +0200, Yann Droneaud wrote:
> Le mercredi 05 septembre 2018 à 16:55 +0100, David Howells a écrit :
> > The size and layout of internal kernel structures may not be relied
> > upon outside of the kernel and may even change in a containerised
> > environment if a container image is frozen and shifted to another
> > machine.
> > 
> > Excise these from Coda's upc_req struct.
...
> 
> This structure should not have been exposed to userspace in the first
> place: it's unusable by userspace as it is. It was incorrect to have it
> outside of #ifdef __KERNEL__ before commit 607ca46e97a1b ... 
...
> So the structure can be moved back to <linux/coda_psdev.h>.

I found a year old patch that clearly fell through the cracks that
fixes this exact thing.

    https://lkml.org/lkml/2017/8/6/186

Jan

Re: [RFC] UAPI: Check headers by compiling all together as C++

[Yann Droneaud]

Hi,

Le mercredi 05 septembre 2018 à 18:55 +0200, Greg KH a écrit :
> On Wed, Sep 05, 2018 at 04:54:27PM +0100, David Howells wrote:
> > 
> > Here's a set of patches that inserts a step into the build process to make
> > sure that the UAPI headers can all be built together with C++ (if the
> > compiler being used supports C++).  All but the final patch perform fixups,
> > including:
> 
> Wait, why do we care?  What has recently changed to start to directly
> import kernel uapi files into C++ code?
> 
> And if userspace wants to do this, can't they do the C namespace trick
> themselves when they do the import?  That must be how they are doing it
> today, right?
> 

They can't.


Adding extern "C" { } doesn't magically make "class" a non keyword.
Even if it was the case, writing C++ code using whatever->class would
probably broke because class is a keyword in C++.

-- 
Yann Droneaud
OPTEYA

Re: [PATCH 03/11] UAPI: virtio_net: Fix use of C++ keywords as structural members

[Michael S. Tsirkin]

On Wed, Sep 05, 2018 at 04:54:55PM +0100, David Howells wrote:
> The virtio_net_ctrl_hdr struct uses a C++ keyword as structural members.  Fix
> this by inserting an anonymous union that provides an alternative name and
> then hide the reserved name in C++.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: "Michael S. Tsirkin" <mst@redhat.com>
> cc: Jason Wang <jasowang@redhat.com>
> cc: virtualization@lists.linux-foundation.org
> ---
> 
>  include/uapi/linux/virtio_net.h |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index a3715a3224c1..967142bc0e05 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -150,7 +150,12 @@ struct virtio_net_hdr_mrg_rxbuf {
>   * command goes in between.
>   */
>  struct virtio_net_ctrl_hdr {
> -	__u8 class;
> +	union {
> +#ifndef __cplusplus
> +		__u8 class;
> +#endif
> +		__u8 _class;
> +	};
>  	__u8 cmd;
>  } __attribute__((packed));


As long as you do not intend to use any classes, how about
simply adding

-Dclass=_class

to your command line?

Seems to work fine with gcc 8.1.1 on Fedora.

-- 
MST

Re: [RFC] UAPI: Check headers by compiling all together as C++

[Michael S. Tsirkin]

On Wed, Sep 05, 2018 at 07:33:38PM +0200, Yann Droneaud wrote:
> Hi,
> 
> Le mercredi 05 septembre 2018 à 18:55 +0200, Greg KH a écrit :
> > On Wed, Sep 05, 2018 at 04:54:27PM +0100, David Howells wrote:
> > > 
> > > Here's a set of patches that inserts a step into the build process to make
> > > sure that the UAPI headers can all be built together with C++ (if the
> > > compiler being used supports C++).  All but the final patch perform fixups,
> > > including:
> > 
> > Wait, why do we care?  What has recently changed to start to directly
> > import kernel uapi files into C++ code?
> > 
> > And if userspace wants to do this, can't they do the C namespace trick
> > themselves when they do the import?  That must be how they are doing it
> > today, right?
> > 
> 
> They can't.
> 
> 
> Adding extern "C" { } doesn't magically make "class" a non keyword.
> Even if it was the case, writing C++ code using whatever->class would
> probably broke because class is a keyword in C++.

I think it's a bug in the language TBH.

> -- 
> Yann Droneaud
> OPTEYA
> 

Re: [RFC] UAPI: Check headers by compiling all together as C++

[David Howells]

Greg KH <greg@kroah.com> wrote:

> > Here's a set of patches that inserts a step into the build process to make
> > sure that the UAPI headers can all be built together with C++ (if the
> > compiler being used supports C++).  All but the final patch perform fixups,
> > including:
> 
> Wait, why do we care?  What has recently changed to start to directly
> import kernel uapi files into C++ code?

There's at least one outstanding bug due to a C++ identifier in the kernel
UAPI headers.

Are you saying you explicitly don't want people to be able to use the kernel
UAPI headers in C++?

> And if userspace wants to do this, can't they do the C namespace trick
> themselves when they do the import?  That must be how they are doing it
> today, right?

No, because there's no such trick (except with the preprocessor).

David

Re: [RFC] UAPI: Check headers by compiling all together as C++

[Jan Engelhardt]

On Wednesday 2018-09-05 18:55, Greg KH wrote:

>On Wed, Sep 05, 2018 at 04:54:27PM +0100, David Howells wrote:
>> 
>> Here's a set of patches that inserts a step into the build process to make
>> sure that the UAPI headers can all be built together with C++ (if the
>> compiler being used supports C++).  All but the final patch perform fixups,
>> including:
>
>Wait, why do we care?  What has recently changed to start to directly
>import kernel uapi files into C++ code?

With C++11, C++ has become a much nicer language to use (for userspace, anyway).

>And if userspace wants to do this, can't they do the C namespace trick
>themselves when they do the import?

The only trick is to use an extra C source file and extensively wrap things.

Re: [PATCH 07/11] UAPI: nilfs2: Fix use of undefined byteswapping functions

[Al Viro]

On Wed, Sep 05, 2018 at 04:55:23PM +0100, David Howells wrote:
>  nilfs_checkpoint_set_##name(struct nilfs_checkpoint *cp)		\
>  {									\
> -	cp->cp_flags = cpu_to_le32(le32_to_cpu(cp->cp_flags) |		\
> +	cp->cp_flags = __cpu_to_le32(__le32_to_cpu(cp->cp_flags) |	\
>  				   (1UL << NILFS_CHECKPOINT_##flag));	\

How about sanitiziung the damn thing to
	cp->cp_flags |= __cpu_to_le32(1UL << NILFS_CHECKPOINT_##flag));

while you are at it?  Or, perhaps, even
#define NILFS2_CP_FLAG(flag) __cpu_to_le32(1UL << NILFS_CHECKPOINT_##flag)

and cp->cp_flags |= NILFS2_CP_FLAG(flag) for this one,

>  }									\
>  static inline void							\
>  nilfs_checkpoint_clear_##name(struct nilfs_checkpoint *cp)		\
>  {									\
> -	cp->cp_flags = cpu_to_le32(le32_to_cpu(cp->cp_flags) &		\
> +	cp->cp_flags = __cpu_to_le32(__le32_to_cpu(cp->cp_flags) &	\
>  				   ~(1UL << NILFS_CHECKPOINT_##flag));	\
	cp->cp_flags &= ~NILFS2_CP_FLAG(flag);
here

>  }									\
>  static inline int							\
>  nilfs_checkpoint_##name(const struct nilfs_checkpoint *cp)		\
>  {									\
> -	return !!(le32_to_cpu(cp->cp_flags) &				\
> +	return !!(__le32_to_cpu(cp->cp_flags) &				\
>  		  (1UL << NILFS_CHECKPOINT_##flag));			\

and !!(cp->cp_flags & NILFS2_CP_FLAG(flag)
here?  Or maybe even make the damn thing bool and lose the !! here...

)>  }
>  

and similar for those:

> @@ -595,20 +596,20 @@ enum {
>  static inline void							\
>  nilfs_segment_usage_set_##name(struct nilfs_segment_usage *su)		\
>  {									\
> -	su->su_flags = cpu_to_le32(le32_to_cpu(su->su_flags) |		\
> +	su->su_flags = __cpu_to_le32(__le32_to_cpu(su->su_flags) |	\
>  				   (1UL << NILFS_SEGMENT_USAGE_##flag));\
>  }									\
>  static inline void							\
>  nilfs_segment_usage_clear_##name(struct nilfs_segment_usage *su)	\
>  {									\
>  	su->su_flags =							\
> -		cpu_to_le32(le32_to_cpu(su->su_flags) &			\
> +		__cpu_to_le32(__le32_to_cpu(su->su_flags) &		\
>  			    ~(1UL << NILFS_SEGMENT_USAGE_##flag));      \
>  }									\
>  static inline int							\
>  nilfs_segment_usage_##name(const struct nilfs_segment_usage *su)	\
>  {									\
> -	return !!(le32_to_cpu(su->su_flags) &				\
> +	return !!(__le32_to_cpu(su->su_flags) &				\
>  		  (1UL << NILFS_SEGMENT_USAGE_##flag));			\
>  }



> @@ -619,15 +620,15 @@ NILFS_SEGMENT_USAGE_FNS(ERROR, error)
>  static inline void
>  nilfs_segment_usage_set_clean(struct nilfs_segment_usage *su)
>  {
> -	su->su_lastmod = cpu_to_le64(0);
> -	su->su_nblocks = cpu_to_le32(0);
> -	su->su_flags = cpu_to_le32(0);
> +	su->su_lastmod = __cpu_to_le64(0);
> +	su->su_nblocks = __cpu_to_le32(0);
> +	su->su_flags = __cpu_to_le32(0);
>  }
>  
>  static inline int
>  nilfs_segment_usage_clean(const struct nilfs_segment_usage *su)
>  {
> -	return !le32_to_cpu(su->su_flags);
> +	return !__le32_to_cpu(su->su_flags);

"Check that after byteswap it becomes 0", is it?  How is that different
from return !su->su_flags; ?

Re: [PATCH 08/11] UAPI: sound: Fix use of u32 and co. in UAPI headers

[Takashi Sakamoto]

Hi,

On Sep 6 2018 00:55, David Howells wrote:
> Fix the use of u32 and co. in UAPI headers as these are not defined.  Switch
> to using the __u32-style equivalents instead.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Jaroslav Kysela <perex@perex.cz>
> cc: Takashi Iwai <tiwai@suse.com>
> cc: alsa-devel@alsa-project.org (moderated for non-subscribers)
> ---
> 
>   include/uapi/sound/skl-tplg-interface.h |  106 ++++++++++++++++---------------
>   1 file changed, 54 insertions(+), 52 deletions(-)

A similar patch was already proposed[1] and has been applied by Mark to
his tree[2]. Your issue (3) is going to be solved soon for v4.19
kernel.

[1] [alsa-devel] [PATCH] uapi: fix sound/skl-tplg-interface.h userspace 
compilation errors
http://mailman.alsa-project.org/pipermail/alsa-devel/2018-August/139392.html
[2] ASoC: uapi: fix sound/skl-tplg-interface.h userspace compilation errors
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/log/?h=for-4.19


Thanks

Takashi Sakamoto

Re: [PATCH 03/11] UAPI: virtio_net: Fix use of C++ keywords as structural members

[David Howells]

Michael S. Tsirkin <mst@redhat.com> wrote:

> As long as you do not intend to use any classes, how about
> simply adding
> 
> -Dclass=_class
> 
> to your command line?

That kind of misses the point;-).  It's not reasonable to expect all userspace
C++ users to do this.

David

Re: [RFC] UAPI: Check headers by compiling all together as C++

[Yann Droneaud]

Le mercredi 05 septembre 2018 à 19:33 +0200, Yann Droneaud a écrit :
> Le mercredi 05 septembre 2018 à 18:55 +0200, Greg KH a écrit :
> > On Wed, Sep 05, 2018 at 04:54:27PM +0100, David Howells wrote:
> > > 
> > > Here's a set of patches that inserts a step into the build
> > > process to make
> > > sure that the UAPI headers can all be built together with C++ (if
> > > the
> > > compiler being used supports C++).  All but the final patch
> > > perform fixups,
> > > including:
> > 
> > Wait, why do we care?  What has recently changed to start to
> > directly
> > import kernel uapi files into C++ code?
> > 
> > And if userspace wants to do this, can't they do the C namespace
> > trick
> > themselves when they do the import?  That must be how they are
> > doing it
> > today, right?
> > 
> 
> They can't.
> 
> 
> Adding extern "C" { } doesn't magically make "class" a non keyword.
> Even if it was the case, writing C++ code using whatever->class would
> probably broke because class is a keyword in C++.
> 

For the record, libX11 has to handle the kink pf issue with C++
keyword:


https://gitlab.freedesktop.org/xorg/lib/libx11/blob/733f64bfeb311c1d040b2f751bfdef9c9d0f89ef/include/X11/Xlib.h#L227

typedef struct {
	XExtData *ext_data;	/* hook for extension to hang data */
	VisualID visualid;	/* visual id of this visual */
#if defined(__cplusplus) || defined(c_plusplus)
	int c_class;		/* C++ class of screen (monochrome, etc.) */
#else
	int class;		/* class of screen (monochrome, etc.) */
#endif
	unsigned long red_mask, green_mask, blue_mask;	/* mask values */
	int bits_per_rgb;	/* log base 2 of distinct color values */
	int map_entries;	/* color map entries */
} Visual;


Regards.

-- 
Yann Droneaud
OPTEYA

Re: [PATCH 05/11] UAPI: coda: Don't use internal kernel structs in UAPI

[David Howells]

Yann Droneaud <ydroneaud@opteya.com> wrote:

> This structure should not have been exposed to userspace in the first
> place: it's unusable by userspace as it is. It was incorrect to have it
> outside of #ifdef __KERNEL__ before commit 607ca46e97a1b ... 
> ...
> All CODA_REQ_* defines internals to kernel side and not exchanged with
> userspace.
> 
> Please move them back to <linux/coda_psdev.h>

Is there any reason coda_psdev.h needs to be in include/linux/ rather than
fs/coda/?

David

Re: [PATCH 08/11] UAPI: sound: Fix use of u32 and co. in UAPI headers

[David Howells]

Takashi Sakamoto <o-takashi@sakamocchi.jp> wrote:

> A similar patch was already proposed[1] and has been applied by Mark to
> his tree[2]. Your issue (3) is going to be solved soon for v4.19
> kernel.

Thanks, I've pulled the branch leading up to that commit into the base of
mine.  It seems the changes were identical:-)

David

Re: [PATCH 05/11] UAPI: coda: Don't use internal kernel structs in UAPI

[Yann Droneaud]

Hi,

Le jeudi 06 septembre 2018 à 08:13 +0100, David Howells a écrit :
> Yann Droneaud <ydroneaud@opteya.com> wrote:
> 
> > This structure should not have been exposed to userspace in the
> > first
> > place: it's unusable by userspace as it is. It was incorrect to
> > have it
> > outside of #ifdef __KERNEL__ before commit 607ca46e97a1b ... 
> > ...
> > All CODA_REQ_* defines internals to kernel side and not exchanged
> > with
> > userspace.
> > 
> > Please move them back to <linux/coda_psdev.h>
> 
> Is there any reason coda_psdev.h needs to be in include/linux/ rather
> than fs/coda/?
> 

It's a valid concern.

At first I thought the first lines (see below) could have been useful
for userspace:

  #define CODA_PSDEV_MAJOR 67
  #define MAX_CODADEVS  5    /* how many do we allow */


But the file was unsuable for a long long time so we can assume it's
usage by userspace is deprecated, then we could remove it from UAPI,
and moves its content back to include/linux.

As one could see include/linux/coda_psdev.h is not used outside of
fs/coda, moving the header here as you suggests seems to be the correct
solution.

Regards.

-- 
Yann Droneaud
OPTEYA

Re: [PATCH 05/11] UAPI: coda: Don't use internal kernel structs in UAPI

[Jan Harkes]

On Thu, Sep 06, 2018 at 01:52:29PM +0200, Yann Droneaud wrote:
> Hi,
> 
> Le jeudi 06 septembre 2018 à 08:13 +0100, David Howells a écrit :
> > Yann Droneaud <ydroneaud@opteya.com> wrote:
> > 
> > > This structure should not have been exposed to userspace in the
> > > first
> > > place: it's unusable by userspace as it is. It was incorrect to
> > > have it
> > > outside of #ifdef __KERNEL__ before commit 607ca46e97a1b ... 
> > > ...
> > > All CODA_REQ_* defines internals to kernel side and not exchanged
> > > with
> > > userspace.
> > > 
> > > Please move them back to <linux/coda_psdev.h>
> > 
> > Is there any reason coda_psdev.h needs to be in include/linux/ rather
> > than fs/coda/?
> > 
> 
> It's a valid concern.
> 
> At first I thought the first lines (see below) could have been useful
> for userspace:
> 
>   #define CODA_PSDEV_MAJOR 67
>   #define MAX_CODADEVS  5    /* how many do we allow */

Nope, userspace just tries to open /dev/cfs0, or a manually configured
alternative. We have only used linux/coda.h, and actually carry our own
copy of that file which is kept in sync manually, which is why there are
all those ifdefs for different systems in there. This all originates
from the time of the 2.1.x kernels when Coda was built externally.

> But the file was unsuable for a long long time so we can assume it's
> usage by userspace is deprecated, then we could remove it from UAPI,
> and moves its content back to include/linux.
> 
> As one could see include/linux/coda_psdev.h is not used outside of
> fs/coda, moving the header here as you suggests seems to be the correct
> solution.

Agreed.

Jan

Re: [PATCH 03/11] UAPI: virtio_net: Fix use of C++ keywords as structural members

[Michael S. Tsirkin]

On Thu, Sep 06, 2018 at 08:09:19AM +0100, David Howells wrote:
> Michael S. Tsirkin <mst@redhat.com> wrote:
> 
> > As long as you do not intend to use any classes, how about
> > simply adding
> > 
> > -Dclass=_class
> > 
> > to your command line?
> 
> That kind of misses the point;-).  It's not reasonable to expect all userspace
> C++ users to do this.
> 
> David

I thought one of the points was that building kernel with c++ catches
some bugs, no?  If the point is to make life easier for c++ userspace
I'm not sure what we can do to be frank. C++ seems to be adding new
keywords with no restraint (C99 did it with inline and restrict too, but
it seems this stopped) so no good way to future-proof code for all
language dialects.

So I'd like to know which are the actual c++ users asking for this - we
can then accomodate the specific version they need.
Meanwhile people can get by with a wrapper along the lines of
#define class _class
#include <linux/virtio_net.h>
#undef class

-- 
MST

Re: [PATCH 05/11] UAPI: coda: Don't use internal kernel structs in UAPI

[David Howells]

Yann Droneaud <ydroneaud@opteya.com> wrote:

> At first I thought the first lines (see below) could have been useful
> for userspace:
> 
>   #define CODA_PSDEV_MAJOR 67
>   #define MAX_CODADEVS  5    /* how many do we allow */

Note that I was asking about include/linux/coda_psdev.h (the internal kernel
header), not include/uapi/linux/coda_psdev.h (the UAPI header).

David

Re: [PATCH 04/11] UAPI: bcache: Fix use of embedded flexible array

[Jan Engelhardt]

On Wed, 05 Sep 2018 16:55:03 +0100, David Howells wrote:
>
>The bkey struct defined by bcache is embedded in the jset struct. However,
>this is illegal in C++ as there's a "flexible array" at the end of the struct.
>Change this to be a 0-length struct instead.
>
>-	__u64	ptr[];
>+	__u64	ptr[0];

As per the C++ standard, it is _also_ illegal to declare an array of size zero.

"""it [the array size expression] shall be a converted constant expression of
type std::size_t and its value shall be greater than zero."""
—http://eel.is/c++draft/dcl.array

That makes both "__u64 ptr[]" and "__u64 ptr[0]" *implementation-specific
extensions*.


3rd party tooling (concerns both C and C++):

Coverity Scan (IIRC) treats "__u64 ptr[0]" as an array of "definitely-zero"
size. Writing to any element will outright flag an out-of-bounds violation.
That is sensible, since only "ptr[]" was standardized.


Conclusion:

So please, do never use __u64 ptr[0].

Re: [PATCH 04/11] UAPI: bcache: Fix use of embedded flexible array

[David Howells]

Jan Engelhardt <jengelh@inai.de> wrote:

> """it [the array size expression] shall be a converted constant expression of
> type std::size_t and its value shall be greater than zero."""
> —http://eel.is/c++draft/dcl.array

Interesting.  You're not actually quoting the full sentence:

	If the constant-expression is present, it shall be a converted
	constant expression of type std​::​size_­t and its value shall be
	greater than zero.

This suggests that:

	__u64 ptr[]

is actually valid since:

	D1 [ constant-expressionopt ] attribute-specifier-seqopt

suggests that the part between the brackets is optional.

David

Re: [PATCH 04/11] UAPI: bcache: Fix use of embedded flexible array

[Jan Engelhardt]

On Tuesday 2018-10-09 17:41, David Howells wrote:

>Jan Engelhardt <jengelh@inai.de> wrote:
>
>> """it [the array size expression] shall be a converted constant expression of
>> type std::size_t and its value shall be greater than zero."""
>> —http://eel.is/c++draft/dcl.array
>
>Interesting.  You're not actually quoting the full sentence:
>
>	If the constant-expression is present, it shall be a converted
>	constant expression of type std​::​size_­t and its value shall be
>	greater than zero.
>
>This suggests that:
>
>	__u64 ptr[]
>
>is actually valid

I think that kind of validity only goes for this kind of standalone 
decl:

	extern int myints[];

but not for []-inside-struct.