All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Joe Perches <joe@perches.com>, Tom Zanussi <zanussi@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: [for-next][PATCH 23/24] string.h: Add strncmp_prefix() helper macro
Date: Fri, 21 Dec 2018 12:56:41 -0500	[thread overview]
Message-ID: <20181221175659.208858193@goodmis.org> (raw)
In-Reply-To: 20181221175618.968519903@goodmis.org

From: Steven Rostedt <rostedt@goodmis.org>

A discussion came up in the trace triggers thread about converting a
bunch of:

 strncmp(str, "const", sizeof("const") - 1)

use cases into a helper macro. It started with:

	strncmp(str, const, sizeof(const) - 1)

But then Joe Perches mentioned that if a const is not used, the
sizeof() will be the size of a pointer, which can be bad. And that
gcc will optimize strlen("const") into "sizeof("const") - 1".

Thinking about this more, a quick grep in the kernel tree found several
(thousands!) of cases that use this construct. A quick grep also
revealed that there's probably several bugs in that use case. Some are
that people forgot the "- 1" (which I found) and others could be that
the constant for the sizeof is different than the constant (although, I
haven't found any of those, but I also didn't look hard).

I figured the best thing to do is to create a helper macro and place it
into include/linux/string.h. And go around and fix all the open coded
versions of it later.

I plan on only applying this patch and updating the tracing hooks for
this merge window. And perhaps use it to fix some of the bugs that were
found.

I was going to just use:

	strncmp(str, prefix, strlen(prefix))

but then realized that "prefix" is used twice, and will break if
someone does something like:

	strncmp_prefix(str, ptr++);

So instead I check with __builtin_constant_p() to see if the second
parameter is truly a constant, which I use sizeof() anyway (why bother
gcc to optimize it, if we already know it's a constant), and copy the
parameter into a local variable and use that local variable for the non
constant part (with strlen).

Link: http://lkml.kernel.org/r/e3e754f2bd18e56eaa8baf79bee619316ebf4cfc.1545161087.git.tom.zanussi@linux.intel.com
Link: http://lkml.kernel.org/r/20181219211615.2298e781@gandalf.local.home

Cc: Joe Perches <joe@perches.com>
Cc: Tom Zanussi <zanussi@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 include/linux/string.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index 27d0482e5e05..a998a53333ef 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -14,6 +14,28 @@ extern void *memdup_user(const void __user *, size_t);
 extern void *vmemdup_user(const void __user *, size_t);
 extern void *memdup_user_nul(const void __user *, size_t);
 
+/*
+ * A common way to test a prefix of a string is to do:
+ *  strncmp(str, prefix, sizeof(prefix) - 1)
+ *
+ * But this can lead to bugs due to typos, or if prefix is a pointer
+ * and not a constant. Instead use strncmp_prefix().
+ */
+#define strncmp_prefix(str, prefix)					\
+	({								\
+		int ____strcmp_prefix_ret____;				\
+		if (__builtin_constant_p(&prefix)) {			\
+			____strcmp_prefix_ret____ =			\
+				strncmp(str, prefix, sizeof(prefix) - 1); \
+		} else {						\
+			typeof(prefix) ____strcmp_prefix____ = prefix;	\
+			____strcmp_prefix_ret____ =			\
+				strncmp(str, ____strcmp_prefix____,	\
+					strlen(____strcmp_prefix____));	\
+		}							\
+		____strcmp_prefix_ret____;				\
+	})
+
 /*
  * Include machine specific inline routines
  */
-- 
2.19.2



  parent reply	other threads:[~2018-12-21 17:57 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-21 17:56 [for-next][PATCH 00/24] tracing: Updates for the next (coming soon) merge window Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 01/24] ftrace: Allow ftrace_replace_code() to be schedulable Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 02/24] arm64: ftrace: Set FTRACE_MAY_SLEEP before ftrace_modify_all_code() Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 03/24] fgraph: Add comment to describe ftrace_graph_get_ret_stack Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 04/24] x86/ftrace: Do not call function graph from dynamic trampolines Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 05/24] powerpc/frace: Use ftrace_graph_get_ret_stack() instead of curr_ret_stack Steven Rostedt
2018-12-21 17:56   ` Steven Rostedt
2018-12-22  9:51   ` Michael Ellerman
2018-12-22  9:51     ` Michael Ellerman
2018-12-22 12:24     ` Steven Rostedt
2018-12-22 12:24       ` Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 06/24] sparc64: " Steven Rostedt
2018-12-21 17:56   ` Steven Rostedt
2018-12-21 18:24   ` David Miller
2018-12-21 18:24     ` David Miller
2018-12-21 19:29     ` Steven Rostedt
2018-12-21 19:29       ` Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 07/24] sh: ftrace: " Steven Rostedt
2018-12-21 17:56   ` Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 08/24] arm64: " Steven Rostedt
2018-12-21 17:56   ` Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 10/24] seq_buf: Use size_t for len in seq_buf_puts() Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 11/24] tracing: Fix ftrace_graph_get_ret_stack() to use task and not current Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 12/24] tracing: Remove unnecessary hist trigger struct field Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 13/24] tracing: Change strlen to sizeof for hist trigger static strings Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 14/24] tracing: Use var_refs[] for hist trigger reference checking Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 15/24] tracing: Remove open-coding of hist trigger var_ref management Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 16/24] tracing: Use hist triggers var_ref array to destroy var_refs Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 17/24] tracing: Remove hist trigger synth_var_refs Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 18/24] tracing: Add hist trigger comments for variable-related fields Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 19/24] tracing: Merge seq_print_sym_short() and seq_print_sym_offset() Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 21/24] tracing: Simplify printfing in seq_print_sym Steven Rostedt
2018-12-21 17:56 ` Steven Rostedt [this message]
2018-12-21 18:51   ` [for-next][PATCH 23/24] string.h: Add strncmp_prefix() helper macro Linus Torvalds
2018-12-21 19:40     ` Steven Rostedt
2018-12-21 20:01       ` Linus Torvalds
2018-12-21 20:35         ` Steven Rostedt
2018-12-21 20:46           ` Joe Perches
2018-12-21 20:54             ` Steven Rostedt
2018-12-21 20:58               ` Andreas Schwab
2018-12-21 21:08                 ` Steven Rostedt
2018-12-21 22:20                   ` Joe Perches
2018-12-21 22:29                     ` Linus Torvalds
2018-12-21 22:58                       ` Steven Rostedt
2018-12-21 22:55                     ` Steven Rostedt
2018-12-23 22:01                     ` Rasmus Villemoes
2018-12-23 22:56                       ` Steven Rostedt
2018-12-23 23:52                         ` Rasmus Villemoes
2018-12-24  0:05                           ` Steven Rostedt
2018-12-23 23:01                       ` Joe Perches
2018-12-21 20:58             ` Steven Rostedt
     [not found]           ` <CAHk-=wjtkvFUuRNZU67KccuUKYHw=pYoDMQJ_9OVDFxOwmK9zQ@mail.gmail.com>
2018-12-21 20:55             ` Steven Rostedt
2018-12-21 22:08               ` Linus Torvalds
2018-12-21 22:48                 ` Steven Rostedt
2018-12-21 22:57                   ` Linus Torvalds
2018-12-21 23:03                     ` Steven Rostedt
2018-12-22 10:20     ` Malcolm Priestley
2018-12-22 12:26       ` Steven Rostedt
2018-12-21 17:56 ` [for-next][PATCH 24/24] tracing: Use strncmp_prefix() helper for histogram code Steven Rostedt
     [not found] ` <20181221175656.827708767@goodmis.org>
2018-12-21 18:00   ` [for-next][PATCH 09/24] seq_buf: Make seq_buf_puts() null-terminate the buffer Steven Rostedt
2018-12-21 18:05     ` Steven Rostedt
2018-12-27  5:25       ` Michael Ellerman

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=20181221175659.208858193@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=zanussi@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.