From: David Howells <dhowells@redhat.com> Date: 2018-09-05 20:25:25
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
From: David Howells <dhowells@redhat.com> Date: 2018-09-05 15:54:46
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 <redacted>
cc: David Airlie <redacted>
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(-)
@@ -148,7 +148,12 @@ struct drm_msm_gem_cpu_fini {*/structdrm_msm_gem_submit_reloc{__u32submit_offset;/* in, offset from submit_bo */-__u32or;/* in, value OR'd with result */+union{+#ifndef __cplusplus+__u32or;/* in, value OR'd with result */+#endif+__u32_or;/* in, value OR'd with result */+};__s32shift;/* in, amount of left shift (can be negative) */__u32reloc_idx;/* in, index of reloc_bo buffer */__u64reloc_offset;/* in, offset from start of reloc_bo */
From: David Howells <dhowells@redhat.com> Date: 2018-09-05 15:54:53
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 <redacted>
cc: keyrings@vger.kernel.org
---
include/uapi/linux/keyctl.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
From: David Howells <dhowells@redhat.com> Date: 2018-09-05 15:55:01
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 <redacted>
cc: virtualization@lists.linux-foundation.org
---
include/uapi/linux/virtio_net.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
From: David Howells <dhowells@redhat.com> Date: 2018-09-05 15:55:07
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 <redacted>
cc: Kent Overstreet <redacted>
cc: linux-bcache@vger.kernel.org
---
include/uapi/linux/bcache.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: David Howells <dhowells@redhat.com> Date: 2018-09-05 15:55:21
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(-)
@@ -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>structipt_ECN_info{__u8operation;/* bitset of operations */
From: David Howells <dhowells@redhat.com> Date: 2018-09-05 15:55:36
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(-)
From: David Howells <dhowells@redhat.com> Date: 2018-09-05 15:55:42
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 <redacted>
cc: Dan Williams <redacted>
cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org
---
include/uapi/linux/ndctl.h | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
From: David Howells <dhowells@redhat.com> Date: 2018-09-05 15:55:47
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 <redacted>
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
From: David Howells <dhowells@redhat.com> Date: 2018-09-05 15:55:55
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 <redacted>
cc: Michal Marek <redacted>
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
On Wed, Sep 05, 2018 at 04:54:55PM +0100, David Howells wrote:
quoted hunk
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 <redacted>
cc: virtualization@lists.linux-foundation.org
---
include/uapi/linux/virtio_net.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
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
Le mercredi 05 septembre 2018 à 16:55 +0100, David Howells a écrit :
quoted hunk
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(+)
@@ -10,14 +10,18 @@/* messages between coda filesystem in kernel and Venus */structupc_req{+#ifdef __KERNEL__structlist_headuc_chain;+#endifcaddr_tuc_data;u_shortuc_flags;u_shortuc_inSize;/* Size is at most 5000 bytes */u_shortuc_outSize;u_shortuc_opcode;/* copied from data to save lookup */intuc_unique;+#ifdef __KERNEL__wait_queue_head_tuc_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
From: David Howells <dhowells@redhat.com> Date: 2018-09-05 17:15:10
Greg KH [off-list ref] 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
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:
quoted
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
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2018-09-05 17:35:33
On Wed, Sep 05, 2018 at 04:54:55PM +0100, David Howells wrote:
quoted hunk
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 <redacted>
cc: virtualization@lists.linux-foundation.org
---
include/uapi/linux/virtio_net.h | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
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
From: Jan Engelhardt <hidden> Date: 2018-09-05 19:22:11
On Wednesday 2018-09-05 18:55, Greg KH wrote:
On Wed, Sep 05, 2018 at 04:54:27PM +0100, David Howells wrote:
quoted
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.
From: David Howells <dhowells@redhat.com> Date: 2018-09-05 20:25:59
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(+)
@@ -10,14 +10,18 @@/* messages between coda filesystem in kernel and Venus */structupc_req{+#ifdef __KERNEL__structlist_headuc_chain;+#endifcaddr_tuc_data;u_shortuc_flags;u_shortuc_inSize;/* Size is at most 5000 bytes */u_shortuc_outSize;u_shortuc_opcode;/* copied from data to save lookup */intuc_unique;+#ifdef __KERNEL__wait_queue_head_tuc_sleep;/* process' wait queue */+#endif};#define CODA_REQ_ASYNC 0x1
From: David Howells <dhowells@redhat.com> Date: 2018-09-05 20:26:14
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 <redacted>
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(-)
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
From: Jan Harkes <jaharkes@cs.cmu.edu> Date: 2018-09-05 21:56:20
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
From: Jan Harkes <jaharkes@cs.cmu.edu> Date: 2018-09-05 22:00:11
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 :
quoted
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>.
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2018-09-05 22:14:19
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 :
quoted
On Wed, Sep 05, 2018 at 04:54:27PM +0100, David Howells wrote:
quoted
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++.
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,
From: David Howells <dhowells@redhat.com> Date: 2018-09-05 22:21:58
Greg KH [off-list ref] wrote:
quoted
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
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(-)
Le mercredi 05 septembre 2018 à 19:33 +0200, Yann Droneaud a écrit :
Le mercredi 05 septembre 2018 à 18:55 +0200, Greg KH a écrit :
quoted
On Wed, Sep 05, 2018 at 04:54:27PM +0100, David Howells wrote:
quoted
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
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
From: David Howells <dhowells@redhat.com> Date: 2018-09-06 11:47:56
Yann Droneaud [off-list ref] 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
From: "Michael S. Tsirkin" <mst@redhat.com> Date: 2018-09-06 14:37:02
On Thu, Sep 06, 2018 at 08:09:19AM +0100, David Howells wrote:
Michael S. Tsirkin [off-list ref] wrote:
quoted
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
From: David Howells <dhowells@redhat.com> Date: 2018-09-06 14:53:53
Yann Droneaud [off-list ref] 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
Hi,
Le jeudi 06 septembre 2018 à 08:13 +0100, David Howells a écrit :
Yann Droneaud [off-list ref] wrote:
quoted
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
From: Jan Harkes <jaharkes@cs.cmu.edu> Date: 2018-09-06 16:51:41
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 :
quoted
Yann Droneaud [off-list ref] wrote:
quoted
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.
From: Jan Engelhardt <hidden> Date: 2018-10-02 14:53:01
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].
From: David Howells <dhowells@redhat.com> Date: 2018-10-09 15:41:19
Jan Engelhardt [off-list ref] 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
From: Jan Engelhardt <hidden> Date: 2018-10-09 16:54:47
On Tuesday 2018-10-09 17:41, David Howells wrote:
Jan Engelhardt [off-list ref] wrote:
quoted
"""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.