All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alejandro Colomar <alx@kernel.org>
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: sashiko-reviews@lists.linux.dev, linux-hardening@vger.kernel.org,
	 devicetree@vger.kernel.org, linux-gpio@vger.kernel.org,
	robh@kernel.org, conor+dt@kernel.org,
	 Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>,
	Andy Shevchenko <andy@kernel.org>
Subject: Re: [PATCH v5 2/7] string: Warn if the prefix is the empty string
Date: Fri, 13 Nov 2026 15:32:31 +0100	[thread overview]
Message-ID: <avce5JRaIf8P6aGT@devuan> (raw)
In-Reply-To: <avcd38TtBtRtWSMe@ashevche-desk.local>

[-- Attachment #1: Type: text/plain, Size: 2188 bytes --]

Hi Andy,

> Date: 2026-11-13 16:25:35+0200
> From: Andy Shevchenko <andriy.shevchenko@intel.com>
>
> On Fri, Nov 13, 2026 at 02:48:01PM +0100, Alejandro Colomar wrote:
> 
> ...
> 
> > > [Severity: Low]
> > > Does moving str_has_prefix() out-of-line into lib/string.c bypass
> > > CONFIG_FORTIFY_SOURCE and prevent compile-time constant folding of
> > > strlen(prefix)?
> > 
> > Preventing the compile-time constant folding of strlen(3) is indeed
> > a performance degradation, but that's necessary to be able to
> > WARN_ON_ONCE(), so there isn't anything we can do.
> > 
> > > lib/string.c is built with __NO_FORTIFY and its top-of-file comment notes
> > > that it is reserved for routines that have architecture-specific
> > > alternatives in <asm/string.h> or get overloaded by FORTIFY_SOURCE, while
> > > other helpers belong in lib/string_helpers.c.
> > 
> > Do we want to move it to "lib/string_helpers.c"?  I don't know enough
> > about the organization in these files, so just let me know what I should
> > do.
> 
> string library (generic) is for broad use, so it should really be clean from
> kernel things. string_helpers is exactly for some custom abstractions we use
> in kernel. Now you should answer these questions (or more than that):
> - Are there or potentially be users that needs this API earlier at the boot?
> - Is this API is detached from the rest of string library semantically?
>   I.o.w. can we group semantically coupled APIs and consider them as a bundle?
> - Due to the above, ask the first Q against semantically grouped APIs.
> 
> My personal feeling that what you are trying to do is no go for generic string
> as is. If you confirm that these APIs are not used / needed at early stages at
> boot, they can be moved completely to string_helpers.

Thanks!  I guess I'll just drop the addition of WARN_ON_ONCE().  Let's
leave the function as is, and document its problems.  It's not worth the
effort.  After all, I think it's a bad function, and should not be used
in new code.


Have a lovely day!
Alex

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2026-11-13 14:32 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-13 13:10 [PATCH v5 0/7] Add strprefix() and fix a bug with it Alejandro Colomar
2026-11-13 13:10 ` [PATCH v5 1/7] string: Add strprefix(), and deprecate str_has_prefix() Alejandro Colomar
2026-11-13 14:33   ` Andy Shevchenko
2026-11-13 14:34     ` Andy Shevchenko
2026-11-13 14:40       ` Alejandro Colomar
2026-11-13 14:47         ` Andy Shevchenko
2026-11-13 13:10 ` [PATCH v5 2/7] string: Warn if the prefix is the empty string Alejandro Colomar
2026-11-13 13:26   ` sashiko-bot
2026-11-13 13:48     ` Alejandro Colomar
2026-11-13 14:25       ` Andy Shevchenko
2026-11-13 14:32         ` Alejandro Colomar [this message]
2026-11-13 13:10 ` [PATCH v5 3/7] device property: Fix fwnode_name_eq() for empty node names Alejandro Colomar
2026-11-13 13:10 ` [PATCH v5 4/7] stddef: Add const_cast() Alejandro Colomar
2026-11-13 13:11 ` [PATCH v5 5/7] string: Add [C]QChar_of() and QVoid_of() Alejandro Colomar
2026-11-13 13:11 ` [PATCH v5 6/7] string: Make strprefix() const-qualifier-generic Alejandro Colomar
2026-11-13 13:11 ` [PATCH v5 7/7] string: Implement strstarts() with strprefix() Alejandro Colomar
2026-11-13 13:20   ` sashiko-bot
2026-11-13 13:36     ` Alejandro Colomar
2026-11-13 14:44   ` Andy Shevchenko
2026-11-13 14:59     ` Alejandro Colomar
2026-11-13 15:07       ` Andy Shevchenko
2026-11-13 15:39         ` Alejandro Colomar
2026-11-13 16:05           ` Andy Shevchenko

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=avce5JRaIf8P6aGT@devuan \
    --to=alx@kernel.org \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=bartosz.golaszewski@oss.qualcomm.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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.