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>,
	Joe Perches <joe@perches.com>, Namhyung Kim <namhyung@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	Rich Felker <dalias@libc.org>,
	linux-sh@vger.kernel.org, Tom Zanussi <zanussi@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: [for-next][PATCH 1/5] string.h: Add str_has_prefix() helper function
Date: Sat, 22 Dec 2018 16:20:08 +0000	[thread overview]
Message-ID: <20181222162856.518489380@goodmis.org> (raw)
In-Reply-To: 20181222162007.697862256@goodmis.org

From: "Steven Rostedt (VMware)" <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.

Note, gcc appears to optimize this when we make it into an always_inline
static function, which removes a lot of issues that a macro produces.

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
Link: http://lkml.kernel.org/r/CAHk-=wg_sR-UEC1ggmkZpypOUYanL5CMX4R7ceuaV4QMf5jBtg@mail.gmail.com

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

diff --git a/include/linux/string.h b/include/linux/string.h
index 27d0482e5e05..7927b875f80c 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -456,4 +456,24 @@ static inline void memcpy_and_pad(void *dest, size_t dest_len,
 		memcpy(dest, src, dest_len);
 }
 
+/**
+ * str_has_prefix - Test if a string has a given prefix
+ * @str: The string to test
+ * @prefix: The string to see if @str starts with
+ *
+ * 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 str_has_prefix().
+ *
+ * Returns: 0 if @str does not start with @prefix
+         strlen(@prefix) if @str does start with @prefix
+ */
+static __always_inline size_t str_has_prefix(const char *str, const char *prefix)
+{
+	size_t len = strlen(prefix);
+	return strncmp(str, prefix, len) = 0 ? len : 0;
+}
+
 #endif /* _LINUX_STRING_H_ */
-- 
2.19.2

WARNING: multiple messages have this Message-ID (diff)
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Joe Perches <joe@perches.com>, Namhyung Kim <namhyung@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Yoshinori Sato <ysato@users.sourceforge.jp>,
	Rich Felker <dalias@libc.org>,
	linux-sh@vger.kernel.org, Tom Zanussi <zanussi@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: [for-next][PATCH 1/5] string.h: Add str_has_prefix() helper function
Date: Sat, 22 Dec 2018 11:20:08 -0500	[thread overview]
Message-ID: <20181222162856.518489380@goodmis.org> (raw)
In-Reply-To: 20181222162007.697862256@goodmis.org

From: "Steven Rostedt (VMware)" <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.

Note, gcc appears to optimize this when we make it into an always_inline
static function, which removes a lot of issues that a macro produces.

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
Link: http://lkml.kernel.org/r/CAHk-=wg_sR-UEC1ggmkZpypOUYanL5CMX4R7ceuaV4QMf5jBtg@mail.gmail.com

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

diff --git a/include/linux/string.h b/include/linux/string.h
index 27d0482e5e05..7927b875f80c 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -456,4 +456,24 @@ static inline void memcpy_and_pad(void *dest, size_t dest_len,
 		memcpy(dest, src, dest_len);
 }
 
+/**
+ * str_has_prefix - Test if a string has a given prefix
+ * @str: The string to test
+ * @prefix: The string to see if @str starts with
+ *
+ * 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 str_has_prefix().
+ *
+ * Returns: 0 if @str does not start with @prefix
+         strlen(@prefix) if @str does start with @prefix
+ */
+static __always_inline size_t str_has_prefix(const char *str, const char *prefix)
+{
+	size_t len = strlen(prefix);
+	return strncmp(str, prefix, len) == 0 ? len : 0;
+}
+
 #endif /* _LINUX_STRING_H_ */
-- 
2.19.2



  reply	other threads:[~2018-12-22 16:20 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-22 16:20 [for-next][PATCH 0/5] tracing: Add string_has_prefix() and usages Steven Rostedt
2018-12-22 16:20 ` Steven Rostedt
2018-12-22 16:20 ` Steven Rostedt [this message]
2018-12-22 16:20   ` [for-next][PATCH 1/5] string.h: Add str_has_prefix() helper function Steven Rostedt
2018-12-22 16:20 ` [for-next][PATCH 2/5] tracing: Use str_has_prefix() helper for histogram code Steven Rostedt
2018-12-22 16:20   ` Steven Rostedt
2018-12-22 18:01   ` Steven Rostedt
2018-12-22 18:01     ` Steven Rostedt
2018-12-22 18:02     ` Steven Rostedt
2018-12-22 18:02       ` Steven Rostedt
2018-12-22 18:03     ` Tom Zanussi
2018-12-22 18:03       ` Tom Zanussi
2018-12-23  3:53       ` Steven Rostedt
2018-12-23  3:53         ` Steven Rostedt
2018-12-23 16:38         ` Tom Zanussi
2018-12-23 16:38           ` Tom Zanussi
2018-12-23  3:32   ` Namhyung Kim
2018-12-23  3:32     ` Namhyung Kim
2018-12-22 16:20 ` [for-next][PATCH 3/5] tracing: Use str_has_prefix() instead of using fixed sizes Steven Rostedt
2018-12-22 16:20   ` Steven Rostedt
2018-12-22 16:20 ` [for-next][PATCH 4/5] tracing: Have the historgram use the result of str_has_prefix() for len of pre Steven Rostedt
2018-12-22 16:20   ` [for-next][PATCH 4/5] tracing: Have the historgram use the result of str_has_prefix() for len of prefix Steven Rostedt
2018-12-23  3:35   ` [for-next][PATCH 4/5] tracing: Have the historgram use the result of str_has_prefix() for len of Namhyung Kim
2018-12-23  3:35     ` [for-next][PATCH 4/5] tracing: Have the historgram use the result of str_has_prefix() for len of prefix Namhyung Kim
2018-12-23 16:40   ` [for-next][PATCH 4/5] tracing: Have the historgram use the result of str_has_prefix() for len of Tom Zanussi
2018-12-23 16:40     ` [for-next][PATCH 4/5] tracing: Have the historgram use the result of str_has_prefix() for len of prefix Tom Zanussi
2018-12-22 16:20 ` [for-next][PATCH 5/5] tracing: Use the return of str_has_prefix() to remove open coded numbers Steven Rostedt
2018-12-22 16:20   ` Steven Rostedt
2018-12-23  3:23   ` Masami Hiramatsu
2018-12-23  3:23     ` Masami Hiramatsu
2018-12-23  3:46     ` Steven Rostedt
2018-12-23  3:46       ` Steven Rostedt
2018-12-23  0:55 ` [for-next][PATCH 0/5] tracing: Add string_has_prefix() and usages Steven Rostedt
2018-12-23  0:55   ` Steven Rostedt
2018-12-23  3:41 ` Namhyung Kim
2018-12-23  3:41   ` Namhyung Kim
2018-12-23  3:53   ` Steven Rostedt
2018-12-23  3:53     ` 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=20181222162856.518489380@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=dalias@libc.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=ysato@users.sourceforge.jp \
    --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.