[alsa-devel] [RFC] UAPI: Check headers by compiling all together as C++
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=uap...
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
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];
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...
Thanks
Takashi Sakamoto
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
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
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++.
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
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/733f64bfeb311c1d040b2f75...
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.
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
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.
participants (6)
-
David Howells -
Greg KH -
Jan Engelhardt -
Michael S. Tsirkin -
Takashi Sakamoto -
Yann Droneaud