[PATCH] Enforce type check in ARRAY_SIZE macro

From: Michał Janiszewski

Inspired by kernel check and recent unification of ARRAY_SIZE, provide
a way of enforcing the macro can only be applied to actual arrays and
not to pointers.

Signed-off-by: Michał Janiszewski 
---
 include/winnt.h | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/include/winnt.h b/include/winnt.h
index 7f822c4aec..ad945464f1 100644
--- a/include/winnt.h
+++ b/include/winnt.h
@@ -760,7 +760,26 @@ typedef struct _MEMORY_BASIC_INFORMATION
   ((type *)((PCHAR)(address) - offsetof(type, field)))

#ifdef __WINESRC__
-# define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+// Validate types used in the expression.
+// Based on https://elixir.bootlin.com/linux/v4.17.4/source/include/linux/kernel.h#L71
+
+#ifdef __GNUC__
+/**
+ * Force a compilation error if condition is true, but also produce a
+ * result (of value 0 and type size_t), so the expression can be used
+ * e.g. in a structure initializer (or where-ever else comma expressions
+ * aren't permitted).
+ */
+#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
+
+/* &a[0] degrades to a pointer: a different type from an array */
+#define __must_be_array(a) \
+    BUILD_BUG_ON_ZERO(__builtin_types_compatible_p(typeof(a), typeof(&a[0])))
+#else
+#define __must_be_array(a) 0
+#endif
+
+# define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) + __must_be_array(x)
 #endif

/* Types */

[Ken Thomases]

On Jul 6, 2018, at 5:25 PM, [e-mail] wrote:
> From: Michał Janiszewski
>
> Inspired by kernel check and recent unification of ARRAY_SIZE, provide
> a way of enforcing the macro can only be applied to actual arrays and
> not to pointers.
> Signed-off-by: Michał Janiszewski
>
> include/winnt.h | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/include/winnt.h b/include/winnt.h
> index 7f822c4aec..ad945464f1 100644
> --- a/include/winnt.h
> +++ b/include/winnt.h
> @@ -760,7 +760,26 @@ typedef struct _MEMORY_BASIC_INFORMATION
>   ((type *)((PCHAR)(address) - offsetof(type, field)))
>
> #ifdef __WINESRC__
> -# define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> +// Validate types used in the expression.
> +// Based on https://elixir.bootlin.com/linux/v4.17.4/source/include/linux/kernel.h#L71 https://elixir.bootlin.com/linux/v4.17.4/source/include/linux/kernel.h#L71

Does that pose a licensing issue, since that code is GPL?
> -
>
> +#ifdef __GNUC__
> +/**
> -
> - Force a compilation error if condition is true, but also produce a
> -
> - result (of value 0 and type size_t), so the expression can be used
> -
> - e.g. in a structure initializer (or where-ever else comma expressions
> -
> - aren't permitted).
> - */
>
> +#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
> -
>
> +/* &a[0] degrades to a pointer: a different type from an array */
> +#define __must_be_array(a) \
> - BUILD_BUG_ON_ZERO(__builtin_types_compatible_p(typeof(a), typeof(&a[0])))

In what version of GCC was __builtin_types_compatible_p() introduced?
> +#else
> +#define __must_be_array(a) 0
> +#endif
> -
>
> +# define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) + __must_be_array(x)

The " + __must_be_array(x)" should be inside the parentheses.
> #endif

-Ken

[Michał Janiszewski]

On 7 July 2018 at 02:03, Ken Thomases  wrote:
> On Jul 6, 2018, at 5:25 PM, [e-mail] wrote:
>
> From: Michał Janiszewski
>
> Inspired by kernel check and recent unification of ARRAY_SIZE, provide
> a way of enforcing the macro can only be applied to actual arrays and
> not to pointers.
> Signed-off-by: Michał Janiszewski
>
> include/winnt.h | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/include/winnt.h b/include/winnt.h
> index 7f822c4aec..ad945464f1 100644
> --- a/include/winnt.h
> +++ b/include/winnt.h
> @@ -760,7 +760,26 @@ typedef struct _MEMORY_BASIC_INFORMATION
>   ((type *)((PCHAR)(address) - offsetof(type, field)))
>
> #ifdef __WINESRC__
> -# define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> +// Validate types used in the expression.
> +// Based on https://elixir.bootlin.com/linux/v4.17.4/source/include/
> linux/kernel.h#L71
>
> Does that pose a licensing issue, since that code is GPL?

I would hope not, but honestly I don't know how to answer this question.
> -
>
> +#ifdef __GNUC__
> +/**
> -
> - Force a compilation error if condition is true, but also produce a
> -
> - result (of value 0 and type size_t), so the expression can be used
> -
> - e.g. in a structure initializer (or where-ever else comma expressions
> -
> - aren't permitted).
> - */
>
> +#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
> -
>
> +/* &a[0] degrades to a pointer: a different type from an array */
> +#define __must_be_array(a) \
> - BUILD_BUG_ON_ZERO(__builtin_types_compatible_p(typeof(a),
>
> typeof(&a[0])))
>
> In what version of GCC was __builtin_types_compatible_p() introduced?

It is already listed in manual for GCC 3.2:
https://gcc.gnu.org/onlinedocs/gcc-3.2/gcc/Other-Builtins.html#Other-Builtin...
> +#else
> +#define __must_be_array(a) 0
> +#endif
> -
>
> +# define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) + __must_be_array(x)
>
> The " + __must_be_array(x)" should be inside the parentheses.

Indeed. How do I send "v2" patch?
> #endif
>
> -Ken

[Ken Thomases]

On Jul 7, 2018, at 8:15 AM, Michał Janiszewski  wrote:
> On 7 July 2018 at 02:03, Ken Thomases <[e-mail] > wrote:
> On Jul 6, 2018, at 5:25 PM, [e-mail]  wrote:
> > +#else
> > +#define __must_be_array(a) 0
> > +#endif
> > -
> >
> > +# define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) + __must_be_array(x)
>
> The " + __must_be_array(x)" should be inside the parentheses.
>
> Indeed. How do I send "v2" patch?

If you use git-send-email, you can add the "-v2" option to change the subject prefix from "PATCH" to "PATCH v2".  It's generally a good idea to add a note explaining what changed from previous versions.  You can use the --annotate option to give you a chance to edit the message before sending it.  You would add a note below the "---" line with an explanation of the change from the previous version.  (There's also the "notes" feature of git for maintaining such explanations, but it's kind of finicky.  See "man git-notes".)

-Ken

[Alex Henrie]

On Sat, Jul 7, 2018 at 6:16 PM Ken Thomases  wrote:
> If you use git-send-email, you can add the "-v2" option to change the subject prefix from "PATCH" to "PATCH v2".

This is actually a feature of the `git format-patch` command, which
you use before `git send-email`. Just wanted to make that clear so
that no one gets confused.

-Alex

[Ken Thomases]

On Jul 7, 2018, at 4:19 PM, Alex Henrie  wrote:
> On Sat, Jul 7, 2018 at 6:16 PM Ken Thomases  wrote:
> > If you use git-send-email, you can add the "-v2" option to change the subject prefix from "PATCH" to "PATCH v2".
>
> This is actually a feature of the `git format-patch` command, which
> you use before `git send-email`. Just wanted to make that clear so
> that no one gets confused.

You _can_ invoke git-format-patch yourself, optionally edit the patch files, and then use git-send-email to send them, but you don't have to.  git-send-email will accept a rev-list and the relevant options and invoke git-format-patch on your behalf.

-Ken

[Henri Verbeet]

On 8 July 2018 at 05:27, Ken Thomases  wrote:
> You _can_ invoke git-format-patch yourself, optionally edit the patch files, and then use git-send-email to send them, but you don't have to.  git-send-email will accept a rev-list and the relevant options and invoke git-format-patch on your behalf.

True, but I think it's good practice to review the patches generated
by format-patch one last time before sending them out with send-email.

[Alex Henrie]

On Sun, Jul 8, 2018 at 2:57 AM Ken Thomases  wrote:
> On Jul 7, 2018, at 4:19 PM, Alex Henrie  wrote:
> > On Sat, Jul 7, 2018 at 6:16 PM Ken Thomases  wrote:
> > > If you use git-send-email, you can add the "-v2" option to change the subject prefix from "PATCH" to "PATCH v2".
> >
> > This is actually a feature of the `git format-patch` command, which
> > you use before `git send-email`. Just wanted to make that clear so
> > that no one gets confused.
>
> You _can_ invoke git-format-patch yourself, optionally edit the patch files, and then use git-send-email to send them, but you don't have to.  git-send-email will accept a rev-list and the relevant options and invoke git-format-patch on your behalf.

Thanks for pointing that out. I didn't realize that git send-email
also accepts the -v2 option.

-Alex