Re: [PATCH v5 7/7] string: Implement strstarts() with strprefix()

[Alejandro Colomar <alx@kernel.org>]

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

> Date: 2026-11-13 13:20:16+0000
> From: sashiko-bot@kernel.org
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Low] string: strstarts() via strprefix() prevents optimizing out
>   pointer offset
> --
> 
> commit 8cdc2e9c96584305ea94313f2618d17d8e28f303
> Author: Alejandro Colomar <alx@kernel.org>
> 
> string: Implement strstarts() with strprefix()
> 
> This commit changes strstarts() to call strprefix() instead of calling
> strncmp() and strlen() directly.
> 
> > diff --git a/include/linux/string.h b/include/linux/string.h
> > index 8c6a79d320474..8ca0deda61606 100644
> > --- a/include/linux/string.h
> > +++ b/include/linux/string.h
> > @@ -595,7 +595,7 @@ static inline const char *strprefix(const char *s, const char *prefix)
> >   */
> >  static inline bool strstarts(const char *str, const char *prefix)
> >  {
> > -	return strncmp(str, prefix, strlen(prefix)) == 0;
> > +	return strprefix(str, prefix);
> >  }
> 
> [Severity: Low]
> Can the compiler actually optimize away the s + len pointer arithmetic in
> strprefix() here?
> 
> strprefix() returns strncmp(s, prefix, len) == 0 ? s + len : NULL, and
> converting that result to bool requires proving that s + len is non-NULL
> whenever strncmp() returns 0.  Because the kernel builds with
> -fno-delete-null-pointer-checks and -fno-strict-overflow, the compiler
> cannot assume that a runtime pointer is non-NULL or that s + len does not
> wrap to NULL, so callers of strstarts() end up keeping str and len live
> across strncmp() and emitting the extra addition and NULL check.

Hmmm, it seems it's right; the compiler can't optimize out.  I hadn't
taken those flags into account.

Here's a test which seems to corroborate:

	alx@devuan:~/tmp$ cat strstarts.c 
	#include <string.h>
	bool strstarts(const char *s, const char *prefix)
	{
		return strncmp(s, prefix, strlen(prefix)) == 0;
	}
	alx@devuan:~/tmp$ cat strprefix.c 
	#include <string.h>
	bool strstarts(const char *s, const char *prefix)
	{
		size_t len = strlen(prefix);
		return strncmp(s, prefix, len) == 0 ? s + len : NULL;
	}
	alx@devuan:~/tmp$ gcc -Wall -Wextra -O2 -fno-delete-null-pointer-checks -fno-strict-overflow -S strstarts.c 
	alx@devuan:~/tmp$ gcc -Wall -Wextra -O2 -fno-delete-null-pointer-checks -fno-strict-overflow -S strprefix.c 
	strprefix.c: In function ‘strstarts’:
	strprefix.c:5:9: warning: the comparison will always evaluate as ‘true’ for the pointer operand in ‘s + (sizetype)len’ must not be NULL [-Waddress]
	    5 |         return strncmp(s, prefix, len) == 0 ? s + len : NULL;
	      |         ^~~~~~
	alx@devuan:~/tmp$ diff -u strstarts.s strprefix.s
	--- strstarts.s	2026-11-13 14:33:48.478848453 +0100
	+++ strprefix.s	2026-11-13 14:33:51.802880696 +0100
	@@ -1,4 +1,4 @@
	-	.file	"strstarts.c"
	+	.file	"strprefix.c"
		.text
		.p2align 4
		.globl	strstarts
	@@ -6,24 +6,45 @@
	 strstarts:
	 .LFB0:
		.cfi_startproc
	-	pushq	%rbx
	+	pushq	%rbp
		.cfi_def_cfa_offset 16
	-	.cfi_offset 3, -16
	+	.cfi_offset 6, -16
	+	pushq	%rbx
	+	.cfi_def_cfa_offset 24
	+	.cfi_offset 3, -24
		movq	%rdi, %rbx
		movq	%rsi, %rdi
	-	subq	$16, %rsp
	-	.cfi_def_cfa_offset 32
	+	subq	$24, %rsp
	+	.cfi_def_cfa_offset 48
		movq	%rsi, 8(%rsp)
		call	strlen@PLT
		movq	8(%rsp), %rsi
		movq	%rbx, %rdi
		movq	%rax, %rdx
	+	movq	%rax, %rbp
		call	strncmp@PLT
		testl	%eax, %eax
	-	sete	%al
	-	addq	$16, %rsp
	+	jne	.L4
	+	addq	%rbp, %rbx
	+	setne	%al
	+	addq	$24, %rsp
	+	.cfi_remember_state
	+	.cfi_def_cfa_offset 24
	+	popq	%rbx
		.cfi_def_cfa_offset 16
	+	popq	%rbp
	+	.cfi_def_cfa_offset 8
	+	ret
	+	.p2align 4,,10
	+	.p2align 3
	+.L4:
	+	.cfi_restore_state
	+	addq	$24, %rsp
	+	.cfi_def_cfa_offset 24
	+	xorl	%eax, %eax
		popq	%rbx
	+	.cfi_def_cfa_offset 16
	+	popq	%rbp
		.cfi_def_cfa_offset 8
		ret
		.cfi_endproc

I'll drop patch 7/7 in v6.

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

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

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