Devicetree
 help / color / mirror / Atom feed
From: Alejandro Colomar <alx@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: devicetree@vger.kernel.org, linux-gpio@vger.kernel.org,
	 robh@kernel.org, conor+dt@kernel.org,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH v3 5/5] string: Make strprefix() const-qualifier-generic
Date: Wed, 11 Nov 2026 01:27:52 +0100	[thread overview]
Message-ID: <avOw9gLozCslpK_D@devuan> (raw)
In-Reply-To: <sashiko-outbox-207284@kernel.org>

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

> Date: 2026-11-10 15:44:36+0000
> From: sashiko-bot@kernel.org
>
> Thank you for your contribution! Sashiko AI review found 3 potential
> issues to consider:
> - [High] string: strprefix() breaks Clang builds via void * subscript in
>   QVoid_of()

This seems nonsense; already replied to in the review of 4/5.

> - [Medium] string: strprefix() always returns char * due to non-const
>   (strprefix)()

Hmmm.  Fixed for v4.

> - [Low] string: strprefix() macro parameter pfx mismatches @prefix
>   kernel-doc

Hmmm.  Fixed for v4.

> --
> 
> commit f882ec908122ee0c3f780648b859ed41700664d0
> Author: Alejandro Colomar <alx@kernel.org>
> 
> string: Make strprefix() const-qualifier-generic
> 
> This commit wraps strprefix() in a const_cast(CQChar_of(s) *, ...) macro so
> that its return type preserves the const qualifier of its first argument.
> 
> > diff --git a/include/linux/string.h b/include/linux/string.h
> > index cb096e9eb9835..3e8a0537b86bb 100644
> > --- a/include/linux/string.h
> > +++ b/include/linux/string.h
> > @@ -577,7 +577,9 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
> >   * *  @s+strlen(@prefix) if @s starts with @prefix
> >   * *  NULL if @s does not start with @prefix
> >   */
> > -static inline char *strprefix(const char *s, const char *prefix)
> > +#define strprefix(s, pfx)  const_cast(CQChar_of(s) *, strprefix(s, pfx))
> 
> [Severity: High]
> Does expanding CQChar_of(s) here break Clang builds (make LLVM=1)?

I can't even seem to build with LLVM=1, so can't reproduce, but as said
in the other subthread (4/5), this makes no sense.  Clang accepts this
code just fine.

> CQChar_of(s) expands to QChar_of(const, s), which expands QVoid_of(s):
> 
> include/linux/string.h:
> #define QVoid_of(p)       typeof((__QVoidptr_of(p)){0}[0])
> #define __QVoidptr_of(p)  typeof(1?(p):(void*){0})
> 
> Because __QVoidptr_of(p) is void * or const void *, applying the [0] array
> subscript operator to it fails to compile under Clang ("error: subscript of a
> pointer to void") in every translation unit that calls strprefix(), such as
> drivers/base/property.c, and triggers -Wpointer-arith under W=3 in GCC.

Hmmmm, I see why it says Clang may not compile.  For that to happen, one
needs to enable non-usual diagnostics.

	alx@devuan:~/tmp$ cat const.c 
	#include <stddef.h>

	#define QVoid_of(p)       typeof((__QVoidptr_of(p)){0}[0])
	#define __QVoidptr_of(p)  typeof(1?(p):(void*){0})

	int
	main(void)
	{
		const void  *p;
		QVoid_of(p) *q;  // const void *
	}
	alx@devuan:~/tmp$ gcc -Wall -Wextra -Wpointer-arith -Wno-unused const.c 
	const.c: In function ‘main’:
	const.c:3:55: warning: pointer of type ‘void *’ used in arithmetic [-Wpointer-arith]
	    3 | #define QVoid_of(p)       typeof((__QVoidptr_of(p)){0}[0])
	      |                                                       ^
	const.c:10:9: note: in expansion of macro ‘QVoid_of’
	   10 |         QVoid_of(p) *q;  // const void *
	      |         ^~~~~~~~

I think this diagnostic isn't very useful, which is why it isn't in
-Wall -Wextra.

> [Severity: Low]
> Does naming the macro parameter pfx instead of prefix cause scripts/kernel-doc
> warnings?  The kernel-doc comment immediately above documents @prefix, and
> scripts/kernel-doc binds that comment to the macro definition that follows it.

Solved.

> > +
> > +static inline char *(strprefix)(const char *s, const char *prefix)
> >  {
> >  	size_t len = strlen(prefix);
> >  	return strncmp(s, prefix, len) == 0 ? (char *) s + len : NULL;
> 
> [Severity: Medium]
> Does strprefix(s, pfx) still unconditionally return char * even when s is
> const char *?
> 
> include/linux/stddef.h:
> #define const_cast(T, p)  _Generic(p, const T: (T) (p), default: (p))
> 
> In const_cast(CQChar_of(s) *, strprefix(s, pfx)), the first _Generic
> association is const CQChar_of(s) *, which is const char *.  Because the
> underlying (strprefix)() function returns char *, the controlling expression
> never matches const char * and always selects the default branch, returning
> an unqualified char * and stripping const from read-only strings.  Should
> (strprefix)() return const char * and avoid casting s to char *?

Solved.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794323750.git.alx@kernel.org?part=5

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

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

  reply	other threads:[~2026-11-11  0:27 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-10 15:33 [PATCH v3 0/5] Add strprefix() and fix a bug with it Alejandro Colomar
2026-11-10 15:33 ` [PATCH v3 1/5] string: Add strprefix(), and deprecate str_has_prefix() Alejandro Colomar
2026-11-10 15:47   ` Joe Perches
2026-11-10 16:14     ` Alejandro Colomar
2026-11-11  8:43   ` Steven Rostedt
2026-11-10 15:33 ` [PATCH v3 2/5] device property: Fix fwnode_name_eq() for empty node names Alejandro Colomar
2026-11-10 15:33 ` [PATCH v3 3/5] stddef: Add const_cast() Alejandro Colomar
2026-11-11 10:25   ` David Laight
2026-11-11 10:51     ` Alejandro Colomar
2026-11-10 15:33 ` [PATCH v3 4/5] string: Add [C]QChar_of() and QVoid_of() Alejandro Colomar
2026-11-10 15:45   ` sashiko-bot
2026-11-11  0:03     ` Alejandro Colomar
2026-11-10 15:33 ` [PATCH v3 5/5] string: Make strprefix() const-qualifier-generic Alejandro Colomar
2026-11-10 15:44   ` sashiko-bot
2026-11-11  0:27     ` Alejandro Colomar [this message]
2026-11-10 16:32   ` Joe Perches
2026-11-10 23:27     ` Alejandro Colomar
2026-11-11 10:28       ` David Laight
2026-11-11 10:59         ` Alejandro Colomar
2026-11-11 10:34   ` David Laight
2026-11-11 11:28     ` Alejandro Colomar

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=avOw9gLozCslpK_D@devuan \
    --to=alx@kernel.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox