2019-02-03 19:24:30

by Rasmus Villemoes

[permalink] [raw]
Subject: [PATCH] build_bug.h: add wrapper for _Static_assert

BUILD_BUG_ON() is a little annoying, since it cannot be used outside
function scope. So one cannot put assertions about the sizeof() a
struct next to the struct definition, but has to hide that in some
more or less arbitrary function.

Since gcc 4.6 (which is now also the required minimum), there is
support for the C11 _Static_assert in all C modes, including gnu89. So
add a simple wrapper for that.

_Static_assert() requires a message argument, which is usually quite
redundant (and I believe that bug got fixed at least in newer C++
standards), but we can easily work around that with a little macro
magic, making it optional.

For example, adding

static_assert(sizeof(struct printf_spec) == 8);

in vsprintf.c and modifying that struct to violate it, one gets

./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct printf_spec) == 8"
#define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")

godbolt.org suggests that _Static_assert() has been support by clang
since at least 3.0.0.

Signed-off-by: Rasmus Villemoes <[email protected]>
---
include/linux/build_bug.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
index faeec7433aab..4bf9ba847b44 100644
--- a/include/linux/build_bug.h
+++ b/include/linux/build_bug.h
@@ -58,4 +58,23 @@
*/
#define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")

+/**
+ * static_assert - check integer constant expression at build time
+ *
+ * static_assert() is a wrapper for the C11 _Static_assert, with a
+ * little macro magic to make the message optional (defaulting to the
+ * stringification of the tested expression).
+ *
+ * Contrary to BUILD_BUG_ON(), static_assert() can be used at global
+ * scope, but requires the expression to be an integer constant
+ * expression (i.e., it is not enough that __builtin_constant_p() is
+ * true for expr).
+ *
+ * Also note that BUILD_BUG_ON() fails the build if the condition is
+ * true, while static_assert() fails the build if the expression is
+ * false.
+ */
+#define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
+#define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
+
#endif /* _LINUX_BUILD_BUG_H */
--
2.20.1



2019-02-04 23:21:53

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] build_bug.h: add wrapper for _Static_assert

On Sun, 3 Feb 2019 20:24:00 +0100 Rasmus Villemoes <[email protected]> wrote:

> BUILD_BUG_ON() is a little annoying, since it cannot be used outside
> function scope. So one cannot put assertions about the sizeof() a
> struct next to the struct definition, but has to hide that in some
> more or less arbitrary function.
>
> Since gcc 4.6 (which is now also the required minimum), there is
> support for the C11 _Static_assert in all C modes, including gnu89. So
> add a simple wrapper for that.
>
> _Static_assert() requires a message argument, which is usually quite
> redundant (and I believe that bug got fixed at least in newer C++
> standards), but we can easily work around that with a little macro
> magic, making it optional.
>
> For example, adding
>
> static_assert(sizeof(struct printf_spec) == 8);
>
> in vsprintf.c and modifying that struct to violate it, one gets
>
> ./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct printf_spec) == 8"
> #define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
>
> godbolt.org suggests that _Static_assert() has been support by clang
> since at least 3.0.0.
>

It would be (very) nice to actually use this macro in a few places so
it gets its build testing while in -next.


2019-02-04 23:22:13

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] build_bug.h: add wrapper for _Static_assert

On Mon, 4 Feb 2019 15:09:16 -0800 Andrew Morton <[email protected]> wrote:

> On Sun, 3 Feb 2019 20:24:00 +0100 Rasmus Villemoes <[email protected]> wrote:
>
> > BUILD_BUG_ON() is a little annoying, since it cannot be used outside
> > function scope. So one cannot put assertions about the sizeof() a
> > struct next to the struct definition, but has to hide that in some
> > more or less arbitrary function.
> >
> > Since gcc 4.6 (which is now also the required minimum), there is
> > support for the C11 _Static_assert in all C modes, including gnu89. So
> > add a simple wrapper for that.
> >
> > _Static_assert() requires a message argument, which is usually quite
> > redundant (and I believe that bug got fixed at least in newer C++
> > standards), but we can easily work around that with a little macro
> > magic, making it optional.
> >
> > For example, adding
> >
> > static_assert(sizeof(struct printf_spec) == 8);
> >
> > in vsprintf.c and modifying that struct to violate it, one gets
> >
> > ./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct printf_spec) == 8"
> > #define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
> >
> > godbolt.org suggests that _Static_assert() has been support by clang
> > since at least 3.0.0.
> >
>
> It would be (very) nice to actually use this macro in a few places so
> it gets its build testing while in -next.

ie, just about every BUILD_BUG_ON in mm/ could use this. Let's switch
a few?


2019-02-05 08:12:46

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] build_bug.h: add wrapper for _Static_assert

On Mon, Feb 4, 2019 at 4:24 AM Rasmus Villemoes
<[email protected]> wrote:
>
> BUILD_BUG_ON() is a little annoying, since it cannot be used outside
> function scope. So one cannot put assertions about the sizeof() a
> struct next to the struct definition, but has to hide that in some
> more or less arbitrary function.
>
> Since gcc 4.6 (which is now also the required minimum), there is
> support for the C11 _Static_assert in all C modes, including gnu89. So
> add a simple wrapper for that.
>
> _Static_assert() requires a message argument, which is usually quite
> redundant (and I believe that bug got fixed at least in newer C++
> standards), but we can easily work around that with a little macro
> magic, making it optional.
>
> For example, adding
>
> static_assert(sizeof(struct printf_spec) == 8);
>
> in vsprintf.c and modifying that struct to violate it, one gets
>
> ./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct printf_spec) == 8"
> #define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
>
> godbolt.org suggests that _Static_assert() has been support by clang
> since at least 3.0.0.
>
> Signed-off-by: Rasmus Villemoes <[email protected]>
> ---
> include/linux/build_bug.h | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
> index faeec7433aab..4bf9ba847b44 100644
> --- a/include/linux/build_bug.h
> +++ b/include/linux/build_bug.h
> @@ -58,4 +58,23 @@
> */
> #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
>
> +/**
> + * static_assert - check integer constant expression at build time
> + *
> + * static_assert() is a wrapper for the C11 _Static_assert, with a
> + * little macro magic to make the message optional (defaulting to the
> + * stringification of the tested expression).
> + *
> + * Contrary to BUILD_BUG_ON(), static_assert() can be used at global
> + * scope, but requires the expression to be an integer constant
> + * expression (i.e., it is not enough that __builtin_constant_p() is
> + * true for expr).
> + *
> + * Also note that BUILD_BUG_ON() fails the build if the condition is
> + * true, while static_assert() fails the build if the expression is
> + * false.
> + */
> +#define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
> +#define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")

What is the "" "" for?


Bikeshed:

There might be room for argument about
where this macro should go.

Another possible place is <linux/compiler.h>
where compiletime_assert() is defined.


Thanks.



--
Best Regards
Masahiro Yamada

2019-02-05 09:38:48

by Rasmus Villemoes

[permalink] [raw]
Subject: Re: [PATCH] build_bug.h: add wrapper for _Static_assert

On 05/02/2019 09.05, Masahiro Yamada wrote:
> On Mon, Feb 4, 2019 at 4:24 AM Rasmus Villemoes
> <[email protected]> wrote:
>>
>> BUILD_BUG_ON() is a little annoying, since it cannot be used outside
>> function scope. So one cannot put assertions about the sizeof() a
>> struct next to the struct definition, but has to hide that in some
>> more or less arbitrary function.
>>
>> Since gcc 4.6 (which is now also the required minimum), there is
>> support for the C11 _Static_assert in all C modes, including gnu89. So
>> add a simple wrapper for that.
>>
>> _Static_assert() requires a message argument, which is usually quite
>> redundant (and I believe that bug got fixed at least in newer C++
>> standards), but we can easily work around that with a little macro
>> magic, making it optional.
>>
>> For example, adding
>>
>> static_assert(sizeof(struct printf_spec) == 8);
>>
>> in vsprintf.c and modifying that struct to violate it, one gets
>>
>> ./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct printf_spec) == 8"
>> #define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
>>
>> godbolt.org suggests that _Static_assert() has been support by clang
>> since at least 3.0.0.
>>
>> Signed-off-by: Rasmus Villemoes <[email protected]>
>> ---
>> include/linux/build_bug.h | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>>
>> diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
>> index faeec7433aab..4bf9ba847b44 100644
>> --- a/include/linux/build_bug.h
>> +++ b/include/linux/build_bug.h
>> @@ -58,4 +58,23 @@
>> */
>> #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
>>
>> +/**
>> + * static_assert - check integer constant expression at build time
>> + *
>> + * static_assert() is a wrapper for the C11 _Static_assert, with a
>> + * little macro magic to make the message optional (defaulting to the
>> + * stringification of the tested expression).
>> + *
>> + * Contrary to BUILD_BUG_ON(), static_assert() can be used at global
>> + * scope, but requires the expression to be an integer constant
>> + * expression (i.e., it is not enough that __builtin_constant_p() is
>> + * true for expr).
>> + *
>> + * Also note that BUILD_BUG_ON() fails the build if the condition is
>> + * true, while static_assert() fails the build if the expression is
>> + * false.
>> + */
>> +#define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
>> +#define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
>
> What is the "" "" for?

Good point. It's a leftover from when I had a fallback-implementation of
_Static_assert for gcc < 4.6, where I wanted to ensure that the second
argument was a string literal, even if my fallback implementation
ignored that argument. Now it's actually a little harmful, because

foobar.c:5:34: error: expected string literal before ‘expected’
static_assert(sizeof(long) == 8, expected 64 bit machine);

is better than

foobar.c:4:34: error: expected ‘)’ before ‘expected’
static_assert(sizeof(long) == 8, expected 64 bit machine);

> Bikeshed:
>
> There might be room for argument about
> where this macro should go.
>
> Another possible place is <linux/compiler.h>
> where compiletime_assert() is defined.

I'd rather move compiletime_assert to build_bug.h, and rename it so that
it becomes an implementation detail of BUILD_BUG. There are not that
many direct users of compiletime_assert(), and I think we should
standardize on fewer ways of achieving the same thing. static_assert()
for checking ICEs, usable at any scope, and BUILD_BUG_* for checking
that the optimizer is sufficiently smart.

This would also be a step towards another cleanup I'd like to do: make
build_bug.h not depend on compiler.h, because we already have a
dependency in the other direction (ARRAY_SIZE using BUILD_BUG_ON_ZERO).

Rasmus

2019-02-05 10:26:59

by Rasmus Villemoes

[permalink] [raw]
Subject: Re: [PATCH] build_bug.h: add wrapper for _Static_assert

On 05/02/2019 00.12, Andrew Morton wrote:
>>
>> It would be (very) nice to actually use this macro in a few places so
>> it gets its build testing while in -next.
>
> ie, just about every BUILD_BUG_ON in mm/ could use this. Let's switch
> a few?
>

Perhaps, but some make sense where they are, e.g. when iterating over
two arrays using one iterator variable, it's fine to assert they have
the same size just above the loop. And, unfortunately, static_assert()
is not quite a drop-in replacement for BUILD_BUG_ON even when the
argument is an ICE due to -Wdeclaration-after-statement.

I'll fix the two BUILD_BUGs I know I've added over the years (one in
vsprintf.c, one in fs/namei.c).

Rasmus

2019-02-05 14:08:51

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] build_bug.h: add wrapper for _Static_assert

On Tue, Feb 5, 2019 at 6:39 PM Rasmus Villemoes
<[email protected]> wrote:
>
> On 05/02/2019 09.05, Masahiro Yamada wrote:
> > On Mon, Feb 4, 2019 at 4:24 AM Rasmus Villemoes
> > <[email protected]> wrote:
> >>
> >> BUILD_BUG_ON() is a little annoying, since it cannot be used outside
> >> function scope. So one cannot put assertions about the sizeof() a
> >> struct next to the struct definition, but has to hide that in some
> >> more or less arbitrary function.
> >>
> >> Since gcc 4.6 (which is now also the required minimum), there is
> >> support for the C11 _Static_assert in all C modes, including gnu89. So
> >> add a simple wrapper for that.
> >>
> >> _Static_assert() requires a message argument, which is usually quite
> >> redundant (and I believe that bug got fixed at least in newer C++
> >> standards), but we can easily work around that with a little macro
> >> magic, making it optional.
> >>
> >> For example, adding
> >>
> >> static_assert(sizeof(struct printf_spec) == 8);
> >>
> >> in vsprintf.c and modifying that struct to violate it, one gets
> >>
> >> ./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct printf_spec) == 8"
> >> #define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
> >>
> >> godbolt.org suggests that _Static_assert() has been support by clang
> >> since at least 3.0.0.
> >>
> >> Signed-off-by: Rasmus Villemoes <[email protected]>
> >> ---
> >> include/linux/build_bug.h | 19 +++++++++++++++++++
> >> 1 file changed, 19 insertions(+)
> >>
> >> diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
> >> index faeec7433aab..4bf9ba847b44 100644
> >> --- a/include/linux/build_bug.h
> >> +++ b/include/linux/build_bug.h
> >> @@ -58,4 +58,23 @@
> >> */
> >> #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
> >>
> >> +/**
> >> + * static_assert - check integer constant expression at build time
> >> + *
> >> + * static_assert() is a wrapper for the C11 _Static_assert, with a
> >> + * little macro magic to make the message optional (defaulting to the
> >> + * stringification of the tested expression).
> >> + *
> >> + * Contrary to BUILD_BUG_ON(), static_assert() can be used at global
> >> + * scope, but requires the expression to be an integer constant
> >> + * expression (i.e., it is not enough that __builtin_constant_p() is
> >> + * true for expr).
> >> + *
> >> + * Also note that BUILD_BUG_ON() fails the build if the condition is
> >> + * true, while static_assert() fails the build if the expression is
> >> + * false.
> >> + */
> >> +#define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
> >> +#define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
> >
> > What is the "" "" for?
>
> Good point. It's a leftover from when I had a fallback-implementation of
> _Static_assert for gcc < 4.6, where I wanted to ensure that the second
> argument was a string literal, even if my fallback implementation
> ignored that argument. Now it's actually a little harmful, because
>
> foobar.c:5:34: error: expected string literal before ‘expected’
> static_assert(sizeof(long) == 8, expected 64 bit machine);
>
> is better than
>
> foobar.c:4:34: error: expected ‘)’ before ‘expected’
> static_assert(sizeof(long) == 8, expected 64 bit machine);
>
> > Bikeshed:
> >
> > There might be room for argument about
> > where this macro should go.
> >
> > Another possible place is <linux/compiler.h>
> > where compiletime_assert() is defined.
>
> I'd rather move compiletime_assert to build_bug.h, and rename it so that
> it becomes an implementation detail of BUILD_BUG. There are not that
> many direct users of compiletime_assert(), and I think we should
> standardize on fewer ways of achieving the same thing. static_assert()
> for checking ICEs, usable at any scope, and BUILD_BUG_* for checking
> that the optimizer is sufficiently smart.
>
> This would also be a step towards another cleanup I'd like to do: make
> build_bug.h not depend on compiler.h,


Probably, you cannot do this.
compiletime_assert relies on __attribute__((__error__(...)))
that is only supported by GCC.


> because we already have a
> dependency in the other direction (ARRAY_SIZE using BUILD_BUG_ON_ZERO).
>
> Rasmus



--
Best Regards
Masahiro Yamada

2019-02-05 20:29:30

by Luc Van Oostenryck

[permalink] [raw]
Subject: Re: [PATCH] build_bug.h: add wrapper for _Static_assert

On Tue, Feb 05, 2019 at 10:53:31AM +0100, Rasmus Villemoes wrote:
> On 05/02/2019 00.12, Andrew Morton wrote:
> >>
> >> It would be (very) nice to actually use this macro in a few places so
> >> it gets its build testing while in -next.
> >
> > ie, just about every BUILD_BUG_ON in mm/ could use this. Let's switch
> > a few?
> >
>
> Perhaps, but some make sense where they are, e.g. when iterating over
> two arrays using one iterator variable, it's fine to assert they have
> the same size just above the loop. And, unfortunately, static_assert()
> is not quite a drop-in replacement for BUILD_BUG_ON even when the
> argument is an ICE due to -Wdeclaration-after-statement.

Yes, unfortunately.

-- Luc

2019-02-07 00:17:44

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] build_bug.h: add wrapper for _Static_assert

On Tue, Feb 5, 2019 at 1:38 AM Rasmus Villemoes
<[email protected]> wrote:
>
> On 05/02/2019 09.05, Masahiro Yamada wrote:
> > On Mon, Feb 4, 2019 at 4:24 AM Rasmus Villemoes
> > <[email protected]> wrote:
> >> +#define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
> >> +#define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")
> >
> > What is the "" "" for?
>
> Good point. It's a leftover from when I had a fallback-implementation of
> _Static_assert for gcc < 4.6, where I wanted to ensure that the second
> argument was a string literal, even if my fallback implementation
> ignored that argument. Now it's actually a little harmful, because

I had assumed it was for the "optional" part of the error message,
since whether you pass a string error message or not, the C
preprocessor should join them all together?

>
> foobar.c:5:34: error: expected string literal before ‘expected’
> static_assert(sizeof(long) == 8, expected 64 bit machine);

Hopefully you'd put `expected` in double quotes?

Note: I'm _very_ happy to see this being added. Once it's landed, I
too can think of some places that it would work better than
BUILD_BUG_ON().
--
Thanks,
~Nick Desaulniers

2019-02-08 20:33:17

by Rasmus Villemoes

[permalink] [raw]
Subject: [PATCH v2 2/3] lib/vsprintf.c: move sizeof(struct printf_spec) next to its definition

At the time of d0484193 (lib/vsprintf.c: expand field_width to 24
bits), there was no compiletime_assert/BUILD_BUG/.... variant that
could be used outside function scope. Now we have static_assert(), so
move the assertion next to the definition instead of hiding it in some
arbitrary function.

Also add the appropriate #include to avoid relying on build_bug.h
being pulled in via some arbitrary chain of includes.

Signed-off-by: Rasmus Villemoes <[email protected]>
---
v2: added, mostly as an example of use of static_assert(), to check
that the compiler actually groks it.

lib/vsprintf.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 3add92329bae..30b00de4f321 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -17,6 +17,7 @@
*/

#include <stdarg.h>
+#include <linux/build_bug.h>
#include <linux/clk.h>
#include <linux/clk-provider.h>
#include <linux/module.h> /* for KSYM_SYMBOL_LEN */
@@ -405,6 +406,8 @@ struct printf_spec {
unsigned int base:8; /* number base, 8, 10 or 16 only */
signed int precision:16; /* # of digits/chars */
} __packed;
+static_assert(sizeof(struct printf_spec) == 8);
+
#define FIELD_WIDTH_MAX ((1 << 23) - 1)
#define PRECISION_MAX ((1 << 15) - 1)

@@ -422,8 +425,6 @@ char *number(char *buf, char *end, unsigned long long num,
int field_width = spec.field_width;
int precision = spec.precision;

- BUILD_BUG_ON(sizeof(struct printf_spec) != 8);
-
/* locase = 0 or 0x20. ORing digits or letters with 'locase'
* produces same digits or (maybe lowercased) letters */
locase = (spec.flags & SMALL);
--
2.20.1


2019-02-08 20:33:19

by Rasmus Villemoes

[permalink] [raw]
Subject: [PATCH v2 1/3] build_bug.h: add wrapper for _Static_assert

BUILD_BUG_ON() is a little annoying, since it cannot be used outside
function scope. So one cannot put assertions about the sizeof() a
struct next to the struct definition, but has to hide that in some
more or less arbitrary function.

Since gcc 4.6 (which is now also the required minimum), there is
support for the C11 _Static_assert in all C modes, including gnu89. So
add a simple wrapper for that.

_Static_assert() requires a message argument, which is usually quite
redundant (and I believe that bug got fixed at least in newer C++
standards), but we can easily work around that with a little macro
magic, making it optional.

For example, adding

static_assert(sizeof(struct printf_spec) == 8);

in vsprintf.c and modifying that struct to violate it, one gets

./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct printf_spec) == 8"
#define __static_assert(expr, msg, ...) _Static_assert(expr, "" msg "")

godbolt.org suggests that _Static_assert() has been support by clang
since at least 3.0.0.

Signed-off-by: Rasmus Villemoes <[email protected]>
---
v2: drop useless "" "" wrapping

include/linux/build_bug.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/include/linux/build_bug.h b/include/linux/build_bug.h
index faeec7433aab..0fe5426f2bdc 100644
--- a/include/linux/build_bug.h
+++ b/include/linux/build_bug.h
@@ -58,4 +58,23 @@
*/
#define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")

+/**
+ * static_assert - check integer constant expression at build time
+ *
+ * static_assert() is a wrapper for the C11 _Static_assert, with a
+ * little macro magic to make the message optional (defaulting to the
+ * stringification of the tested expression).
+ *
+ * Contrary to BUILD_BUG_ON(), static_assert() can be used at global
+ * scope, but requires the expression to be an integer constant
+ * expression (i.e., it is not enough that __builtin_constant_p() is
+ * true for expr).
+ *
+ * Also note that BUILD_BUG_ON() fails the build if the condition is
+ * true, while static_assert() fails the build if the expression is
+ * false.
+ */
+#define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
+#define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
+
#endif /* _LINUX_BUILD_BUG_H */
--
2.20.1


2019-02-08 20:34:43

by Rasmus Villemoes

[permalink] [raw]
Subject: [PATCH v2 3/3] linux/fs.h: move member alignment check next to definition of struct filename

Instead of doing this compile-time check in some slightly arbitrary
user of struct filename, put it next to the definition.

Signed-off-by: Rasmus Villemoes <[email protected]>
---
v2: added, mostly as an example of use of static_assert(), to check
that the compiler actually groks it. Feel free to NAK it as useless
churn.

fs/namei.c | 2 --
include/linux/fs.h | 3 +++
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 914178cdbe94..d604f6b3bcc3 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -39,7 +39,6 @@
#include <linux/bitops.h>
#include <linux/init_task.h>
#include <linux/uaccess.h>
-#include <linux/build_bug.h>

#include "internal.h"
#include "mount.h"
@@ -131,7 +130,6 @@ getname_flags(const char __user *filename, int flags, int *empty)
struct filename *result;
char *kname;
int len;
- BUILD_BUG_ON(offsetof(struct filename, iname) % sizeof(long) != 0);

result = audit_reusename(filename);
if (result)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 29d8e2cfed0e..8dce6932e620 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -37,6 +37,8 @@
#include <linux/uuid.h>
#include <linux/errseq.h>
#include <linux/ioprio.h>
+#include <linux/build_bug.h>
+#include <linux/stddef.h>

#include <asm/byteorder.h>
#include <uapi/linux/fs.h>
@@ -2487,6 +2489,7 @@ struct filename {
struct audit_names *aname;
const char iname[];
};
+static_assert(offsetof(struct filename, iname) % sizeof(long) == 0);

extern long vfs_truncate(const struct path *, loff_t);
extern int do_truncate(struct dentry *, loff_t start, unsigned int time_attrs,
--
2.20.1