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: [RFC][PATCH] libtraceeval: Add size checks to insert and query functions
Date: Tue, 15 Aug 2023 20:02:34 -0400	[thread overview]
Message-ID: <20230815200234.32fd17dd@gandalf.local.home> (raw)

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

Currently, there's nothing that checks the size of the keys/values being
passed into traceeval_query() and traceeval_insert(). Make functions that
take the size of those arrays:

   traceeval_query_size()
   traceeval_insert_size()

and convert the traceeval_query() and traceeval_insert() into macros that
calculate the size of the fields to pass in:

  #define traceeval_query(teval, keys, results) \
     traceeval_query_size(teval, sizeof(keys) / sizeof(keys[0]), results)

and this will allow the code to check to make sure the size matches what
is expected (note, that currently is not done, but can be added).

If the keys or vals is not an array, but instead a pointer, if one of the
macros is used, the compiler will complain with:

   warning: division sizeof (const struct traceeval_data *) / sizeof (const struct traceeval_data)
            does not compute the number of array elements [-Wsizeof-pointer-div]
  190 |                              sizeof(keys) / sizeof(keys[0]), results);

Then the user would need to call the "_size()" function directly
(as is done in the sample code, where the keys received by an iterator is
 used for a query)

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---

  This is an RFC patch. I wonder if we should have this to make the code more
  robust?

 include/traceeval-hist.h | 18 +++++++++++++-----
 samples/task-eval.c      |  2 +-
 src/histograms.c         | 10 +++++-----
 3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/include/traceeval-hist.h b/include/traceeval-hist.h
index d0bee7c02aa0..c8c27a51c62b 100644
--- a/include/traceeval-hist.h
+++ b/include/traceeval-hist.h
@@ -171,15 +171,23 @@ struct traceeval *traceeval_init(struct traceeval_type *keys,
 
 void traceeval_release(struct traceeval *teval);
 
-int traceeval_insert(struct traceeval *teval,
-		     const struct traceeval_data *keys,
-		     const struct traceeval_data *vals);
+int traceeval_insert_size(struct traceeval *teval,
+			  const struct traceeval_data *keys, size_t nr_keys,
+			  const struct traceeval_data *vals, size_t nr_vals);
+
+#define traceeval_insert(teval, keys, vals)				\
+	traceeval_insert_size(teval, keys, sizeof(keys) / sizeof(keys[0]), \
+			      vals, sizeof(vals) / sizeof(vals[0]))
 
 int traceeval_remove(struct traceeval *teval,
 		     const struct traceeval_data *keys);
 
-int traceeval_query(struct traceeval *teval, const struct traceeval_data *keys,
-		    const struct traceeval_data **results);
+int traceeval_query_size(struct traceeval *teval, const struct traceeval_data *keys,
+			 size_t nr_keys, const struct traceeval_data **results);
+
+#define traceeval_query(teval, keys, results)				\
+	traceeval_query_size(teval, keys,				\
+			     sizeof(keys) / sizeof(keys[0]), results);
 
 void traceeval_results_release(struct traceeval *teval,
 			       const struct traceeval_data *results);
diff --git a/samples/task-eval.c b/samples/task-eval.c
index c2950b2d64b0..4eaab7ad6e15 100644
--- a/samples/task-eval.c
+++ b/samples/task-eval.c
@@ -814,7 +814,7 @@ static void display_processes(struct traceeval *teval,
 		struct process_data *pdata = NULL;
 		const char *comm = keys[0].cstring;
 
-		ret = traceeval_query(teval_data, keys, &results);
+		ret = traceeval_query(teval_data, keys, 2, &results);
 		if (ret < 1)
 			continue; /* ?? */
 
diff --git a/src/histograms.c b/src/histograms.c
index a7d0643cfbbd..f71a2d557e34 100644
--- a/src/histograms.c
+++ b/src/histograms.c
@@ -669,8 +669,8 @@ fail:
  *
  * Returns 1 if found, 0 if not found, and -1 on error.
  */
-int traceeval_query(struct traceeval *teval, const struct traceeval_data *keys,
-		    const struct traceeval_data **results)
+int traceeval_query_size(struct traceeval *teval, const struct traceeval_data *keys,
+			 size_t nr_keys, const struct traceeval_data **results)
 {
 	struct entry *entry;
 	int check;
@@ -909,9 +909,9 @@ unsigned long long traceeval_stat_count(struct traceeval_stat *stat)
  *
  * Returns 0 on success, and -1 on error.
  */
-int traceeval_insert(struct traceeval *teval,
-		     const struct traceeval_data *keys,
-		     const struct traceeval_data *vals)
+int traceeval_insert_size(struct traceeval *teval,
+			  const struct traceeval_data *keys, size_t nr_keys,
+			  const struct traceeval_data *vals, size_t nr_vals)
 {
 	struct entry *entry;
 	int check;
-- 
2.40.1


             reply	other threads:[~2023-08-16  0:03 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-16  0:02 Steven Rostedt [this message]
2023-08-16  1:04 ` [RFC][PATCH] libtraceeval: Add size checks to insert and query functions Steven Rostedt
2023-08-16  1:10 ` Steven Rostedt
2023-08-16  1:15   ` Steven Rostedt

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=20230815200234.32fd17dd@gandalf.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).