Linux-Trace-Devel Archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Linux Trace Devel <linux-trace-devel@vger.kernel.org>
Cc: Ross Zwisler <zwisler@google.com>,
	Stevie Alvarez  <stevie.6strings@gmail.com>
Subject: [PATCH v2] libtraceeval: Have TRACEEVAL_ARRAY_SIZE() handle NULL pointer
Date: Thu, 5 Oct 2023 21:22:33 -0400	[thread overview]
Message-ID: <20231005212233.22ae4e13@rorschach.local.home> (raw)

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

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.

			})))

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Changes since v1: https://lore.kernel.org/all/20231005205129.6d6cbfad@gandalf.local.home/

- 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


             reply	other threads:[~2023-10-06  1:22 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-06  1:22 Steven Rostedt [this message]
2023-10-11 22:33 ` [PATCH v2] libtraceeval: Have TRACEEVAL_ARRAY_SIZE() handle NULL pointer Ross Zwisler
2023-10-11 23:14   ` Steven Rostedt
2023-10-17 14:47     ` Ross Zwisler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231005212233.22ae4e13@rorschach.local.home \
    --to=rostedt@goodmis.org \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=stevie.6strings@gmail.com \
    --cc=zwisler@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).