From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 55E55C43387 for ; Fri, 21 Dec 2018 23:19:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 091B721929 for ; Fri, 21 Dec 2018 23:19:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2392179AbeLUXTh (ORCPT ); Fri, 21 Dec 2018 18:19:37 -0500 Received: from smtprelay0071.hostedemail.com ([216.40.44.71]:59568 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725372AbeLUXTh (ORCPT ); Fri, 21 Dec 2018 18:19:37 -0500 Received: from filter.hostedemail.com (clb03-v110.bra.tucows.net [216.40.38.60]) by smtprelay03.hostedemail.com (Postfix) with ESMTP id 3E8F2837F243; Fri, 21 Dec 2018 23:19:36 +0000 (UTC) X-Session-Marker: 6A6F6540706572636865732E636F6D X-HE-Tag: bulb00_368440967965b X-Filterd-Recvd-Size: 4719 Received: from XPS-9350.home (unknown [47.151.153.53]) (Authenticated sender: joe@perches.com) by omf08.hostedemail.com (Postfix) with ESMTPA; Fri, 21 Dec 2018 23:19:34 +0000 (UTC) Message-ID: Subject: Re: [PATCH v3] string.h: Add str_has_prefix() helper From: Joe Perches To: Steven Rostedt , LKML Cc: Linus Torvalds , Ingo Molnar , Andrew Morton , Greg Kroah-Hartman , Namhyung Kim , Masami Hiramatsu , Tom Zanussi Date: Fri, 21 Dec 2018 15:19:33 -0800 In-Reply-To: <20181221181316.6d32bdc8@gandalf.local.home> References: <20181221181316.6d32bdc8@gandalf.local.home> Content-Type: text/plain; charset="ISO-8859-1" User-Agent: Evolution 3.30.1-1build1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2018-12-21 at 18:13 -0500, Steven Rostedt wrote: > From: "Steven Rostedt (VMware)" > > 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 > Cc: Greg Kroah-Hartman > Suggestions-by: Linus Torvalds > Suggestions-by: Joe Perches > Suggestions-by: Andreas Schwab > Signed-off-by: Steven Rostedt (VMware) > --- > > Changes since v2: > > - Rename the helper function to str_has_prefix() > (Linus Torvalds and Joe Perches) > > - Use a static inline, as that appears to still optimize correctly > (Andreas Schwab) > > include/linux/string.h | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/include/linux/string.h b/include/linux/string.h > index 27d0482e5e05..a37859d91b57 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 int str_has_prefix(const char *str, const char *prefix) > +{ > + int len = strlen(prefix); > + return strncmp(str, prefix, len) == 0 ? len : 0; > +} I believe this should be bool. I don't find a use for non-zero assigned len value in the kernel for strncmp and I believe the function should simply be: static inline bool str_has_prefix(const char *str, const char prefix[]) { return !strncmp(str, prefix, strlen(prefix)); } It's hard to believe __always_inline vs inline matters for any single line function.