Skip to contentSkip to conversation

sh: use sizeof() in memchunk_cmdline_override

PATCH In review Thorsten Blum 2 participants 4 messages Apr 23 – Apr 24 SuperH

lore
Share & tools
lore

Thorsten Blum

author
#0 lore raw

PATCH 1/1 sh: use sizeof() in memchunk_cmdline_override

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

Signed-off-by Thorsten Blum <[email protected]>
---
 arch/sh/mm/consistent.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/sh/mm/consistent.c +1 -1
index 0de206c1acfe..8f3a5bcbccfe 100644
@@ -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);
#1 lore raw

Hi Thorsten,

Quoted 4 lines · Thorsten Blum

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

Signed-off-by Thorsten Blum <[email protected]>

Thanks for your patch!

arch/sh/mm/consistent.c static void __init memchunk_cmdline_override(char *name, unsigned long *sizep) · line 26

Quoted 8 lines
arch/sh/mm/consistent.c +1 -1
@@ -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?

arch/sh/mm/consistent.c static void __init memchunk_cmdline_override(char *name, unsigned long *sizep) · line 29

Quoted 3 lines
arch/sh/mm/consistent.c +0 -0
                if (!strncmp(name, p, k) && p[k] == '=') {
                        p += k + 1;
                        *sizep = memparse(p, NULL);

Gr{oetje,eeting}s,

Geert

Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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
author
#2 lore raw
Quoted 15 lines · Geert Uytterhoeven

Hi Thorsten,

Quoted 4 lines · Thorsten Blum

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

Signed-off-by Thorsten Blum <[email protected]>

Thanks for your patch!

Quoted 5 lines
arch/sh/mm/consistent.c +0 -0
@@ -23,7 +23,7 @@ static void __init memchunk_cmdline_override(char *name, unsigned long *sizep)
        int k = strlen(name);
Quoted 6 lines

arch/sh/mm/consistent.c static void __init memchunk_cmdline_override(char *name, unsigned long *sizep) · line 26

Quoted 3 lines
arch/sh/mm/consistent.c +1 -1
        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().

author
#3 lore raw
Quoted 23 lines · Thorsten Blum
Quoted 15 lines · Geert Uytterhoeven

Hi Thorsten,

Quoted 4 lines · Thorsten Blum

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

Signed-off-by Thorsten Blum <[email protected]>

Thanks for your patch!

Quoted 5 lines
arch/sh/mm/consistent.c +0 -0
@@ -23,7 +23,7 @@ static void __init memchunk_cmdline_override(char *name, unsigned long *sizep)
        int k = strlen(name);
Quoted 6 lines

arch/sh/mm/consistent.c static void __init memchunk_cmdline_override(char *name, unsigned long *sizep) · line 26

Quoted 3 lines
arch/sh/mm/consistent.c +1 -1
        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?

Quoted 2 lines

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.