diff mbox series

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

Message ID 153616290368.23468.7806230605345568524.stgit@warthog.procyon.org.uk (mailing list archive)
State New, archived
Headers show
Series [01/11] UAPI: drm: Fix use of C++ keywords as structural members | expand

Commit Message

David Howells Sept. 5, 2018, 3:55 p.m. UTC
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(-)

Comments

Jan Engelhardt Oct. 2, 2018, 2:52 p.m. UTC | #1
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].
David Howells Oct. 9, 2018, 3:41 p.m. UTC | #2
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
Jan Engelhardt Oct. 9, 2018, 4:54 p.m. UTC | #3
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.
diff mbox series

Patch

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)				\