[PATCH] sh: use sizeof() in memchunk_cmdline_override

[PATCH] sh: use sizeof() in memchunk_cmdline_override

[Thorsten Blum]

Replace the hard-coded string length with 'sizeof("memchunk.") - 1' and
remove the comment.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 arch/sh/mm/consistent.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sh/mm/consistent.c b/arch/sh/mm/consistent.c
index 0de206c1acfe..8f3a5bcbccfe 100644
--- a/arch/sh/mm/consistent.c
+++ b/arch/sh/mm/consistent.c
@@ -23,7 +23,7 @@ static void __init memchunk_cmdline_override(char *name, unsigned long *sizep)
 	int k = strlen(name);
 
 	while ((p = strstr(p, "memchunk."))) {
-		p += 9; /* strlen("memchunk.") */
+		p += sizeof("memchunk.") - 1;
 		if (!strncmp(name, p, k) && p[k] == '=') {
 			p += k + 1;
 			*sizep = memparse(p, NULL);

Re: [PATCH] sh: use sizeof() in memchunk_cmdline_override

[Geert Uytterhoeven]

Hi Thorsten,

On Thu, 23 Apr 2026 at 14:10, Thorsten Blum <thorsten.blum@linux.dev> wrote:
> Replace the hard-coded string length with 'sizeof("memchunk.") - 1' and
> remove the comment.
>
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>

Thanks for your patch!

> --- a/arch/sh/mm/consistent.c
> +++ b/arch/sh/mm/consistent.c
> @@ -23,7 +23,7 @@ static void __init memchunk_cmdline_override(char *name, unsigned long *sizep)
>         int k = strlen(name);
>
>         while ((p = strstr(p, "memchunk."))) {
> -               p += 9; /* strlen("memchunk.") */
> +               p += sizeof("memchunk.") - 1;

Can't you just use strlen() instead, i.e. won't the compiler optimize
that into a constant?

>                 if (!strncmp(name, p, k) && p[k] == '=') {
>                         p += k + 1;
>                         *sizep = memparse(p, NULL);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] sh: use sizeof() in memchunk_cmdline_override

[Thorsten Blum]

On Fri, Apr 24, 2026 at 01:05:48PM +0200, Geert Uytterhoeven wrote:
> Hi Thorsten,
> 
> On Thu, 23 Apr 2026 at 14:10, Thorsten Blum <thorsten.blum@linux.dev> wrote:
> > Replace the hard-coded string length with 'sizeof("memchunk.") - 1' and
> > remove the comment.
> >
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> 
> Thanks for your patch!
> 
> > --- a/arch/sh/mm/consistent.c
> > +++ b/arch/sh/mm/consistent.c
> > @@ -23,7 +23,7 @@ static void __init memchunk_cmdline_override(char *name, unsigned long *sizep)
> >         int k = strlen(name);
> >
> >         while ((p = strstr(p, "memchunk."))) {
> > -               p += 9; /* strlen("memchunk.") */
> > +               p += sizeof("memchunk.") - 1;
> 
> Can't you just use strlen() instead, i.e. won't the compiler optimize
> that into a constant?

That only works if ARCH_HAS_FORTIFY_SOURCE is selected, which is not the
case for arch/sh. Otherwise I would have used strlen().

Re: [PATCH] sh: use sizeof() in memchunk_cmdline_override

[Thorsten Blum]

On Fri, Apr 24, 2026 at 01:15:20PM +0200, Thorsten Blum wrote:
> On Fri, Apr 24, 2026 at 01:05:48PM +0200, Geert Uytterhoeven wrote:
> > Hi Thorsten,
> > 
> > On Thu, 23 Apr 2026 at 14:10, Thorsten Blum <thorsten.blum@linux.dev> wrote:
> > > Replace the hard-coded string length with 'sizeof("memchunk.") - 1' and
> > > remove the comment.
> > >
> > > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > 
> > Thanks for your patch!
> > 
> > > --- a/arch/sh/mm/consistent.c
> > > +++ b/arch/sh/mm/consistent.c
> > > @@ -23,7 +23,7 @@ static void __init memchunk_cmdline_override(char *name, unsigned long *sizep)
> > >         int k = strlen(name);
> > >
> > >         while ((p = strstr(p, "memchunk."))) {
> > > -               p += 9; /* strlen("memchunk.") */
> > > +               p += sizeof("memchunk.") - 1;
> > 
> > Can't you just use strlen() instead, i.e. won't the compiler optimize
> > that into a constant?
> 
> That only works if ARCH_HAS_FORTIFY_SOURCE is selected, which is not the
> case for arch/sh. Otherwise I would have used strlen().

More concretely, because __HAVE_ARCH_STRLEN is set in
arch/sh/include/asm/string_32.h, strlen() resolves to the arch-specific
assembly implementation in arch/sh/lib/strlen.S.

However, constant folding is only guaranteed when calls go through
__builtin_strlen(), e.g. via the wrapper in linux/fortify-string.h,
which SH does not use with dreamcast_defconfig.

I also checked the disassembly, and sh4-linux-gnu-gcc (Debian 14.2.0-19)
did not optimize it into a constant.