On Thu, Oct 05, 2023 at 09:22:33PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@xxxxxxxxxxx>
>
> In the new addition to make sure that pointers passed to traceeval_init()
> and other functions that require a static array and not a dynamic one will
> cause the build to fail, this causes NULL pointers to fail the build too.
>
> Although keys must be filled, vals are allowed to be NULL. It was assumed
> that:
>
> (void *)vals == NULL ? TRACEEVAL_ARRAY_SIZE(vals))
>
> Would solve this, but it gcc was actually still giving a warning about
>
> warning: division 'sizeof (void *) / sizeof (void)' does not compute the number of array elements
>
> But now it actually fails to build with the magic check.
>
> Change TRACEEVAL_ARRAY_SIZE() to handle NULL for both keys and vals, by
> not only having:
>
> #define TRACEEVAL_ARRAY_SIZE(data) \
> ((void *)(data) == NULL ? 0 : __TRACEEVAL_ARRAY_SIZE(data))
>
> But that is not enough, as gcc still evaluates the second part, and it
> will fail to build. To handle this, change that to:
>
> #define __TRACEEVAL_ARRAY_SIZE(data) \
> ((sizeof(data) / (sizeof((data)[0])) + 0) + \
>
> The above adds " + 0" to the "sizeof((data)[0])" which quiets the warning
> mentioned above (the addition of zero breaks the normal pattern of finding
> an array size).
>
> (int)(sizeof(struct { \
> int:(-!!(__builtin_types_compatible_p(typeof(data), \
> typeof(&((data)[0]))) && \
> (void *)(data) != NULL)); \
>
> Added "&& (void *)(data) != NULL" that makes the above return false (zero)
> for a static array and NULL, which is exactly what we want.
Don't we already know it's not NULL because of the check in
TRACEEVAL_ARRAY_SIZE()? Or do we really need to check for NULL in both
macros?
>
> })))
A few random parens in the changelog. :)
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> ---
> Changes since v1: https://lore.kernel.org/all/20231005205129.6d6cbfad@xxxxxxxxxxxxxxxxxx/
>
> - Fixed error in placement of parenthesis.
>
> include/traceeval.h | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/include/traceeval.h b/include/traceeval.h
> index bbf8f6ac7dd1..d8095ba5c1a1 100644
> --- a/include/traceeval.h
> +++ b/include/traceeval.h
> @@ -29,13 +29,17 @@
> *
> * struct traceeval_data keys[] = { ... };
> */
> -#define TRACEEVAL_ARRAY_SIZE(data) \
> - ((sizeof(data) / sizeof((data)[0])) + \
> +#define __TRACEEVAL_ARRAY_SIZE(data) \
> + ((sizeof(data) / (sizeof((data)[0]) + 0)) + \
> (int)(sizeof(struct { \
> int:(-!!(__builtin_types_compatible_p(typeof(data), \
> - typeof(&((data)[0]))))); \
> + typeof(&((data)[0]))) && \
> + (void *)(data) != NULL)); \
> })))
>
> +#define TRACEEVAL_ARRAY_SIZE(data) \
> + ((void *)(data) == NULL ? 0 : __TRACEEVAL_ARRAY_SIZE(data))
> +
> /* Data type distinguishers */
> enum traceeval_data_type {
> TRACEEVAL_TYPE_NONE,
> @@ -191,7 +195,7 @@ struct traceeval;
> #define traceeval_init(keys, vals) \
> traceeval_init_size(keys, vals, \
> TRACEEVAL_ARRAY_SIZE(keys), \
> - (void *)vals == NULL ? 0 : TRACEEVAL_ARRAY_SIZE(vals))
> + TRACEEVAL_ARRAY_SIZE(vals))
>
> #define traceeval_init_size(keys, vals, nr_keys, nr_vals) \
> traceeval_init_data_size(keys, vals, nr_keys, nr_vals, \
> @@ -211,7 +215,7 @@ int traceeval_insert_size(struct traceeval *teval,
>
> #define traceeval_insert(teval, keys, vals) \
> traceeval_insert_size(teval, keys, TRACEEVAL_ARRAY_SIZE(keys), \
> - vals, (void *)vals == NULL ? 0 : TRACEEVAL_ARRAY_SIZE(vals))
> + vals, TRACEEVAL_ARRAY_SIZE(vals))
>
> int traceeval_remove_size(struct traceeval *teval,
> const struct traceeval_data *keys, size_t nr_keys);
> --
> 2.40.1
>
|