[PATCH 0/2] sh: use generic strncpy()

[PATCH 0/2] sh: use generic strncpy()

[Kuninori Morimoto]

Hi

These are strncpy() fixup patches, and both are crated by Karl.
He created 2 patterns.

Mine (= Kuninori Morimoto) and/or His patches can solve
issue, but we can follow maintainer's slection

Thank you for your help !!
Best regards
---
Kuninori Morimoto

[PATCH] sh: use generic strncpy()

[Kuninori Morimoto]

From: Karl Nasrallah <knnspeed@aol.com>

Current SH will get below warning at strncpy()

In file included from ${LINUX}/arch/sh/include/asm/string.h:3,
                 from ${LINUX}/include/linux/string.h:20,
                 from ${LINUX}/include/linux/bitmap.h:9,
                 from ${LINUX}/include/linux/nodemask.h:95,
                 from ${LINUX}/include/linux/mmzone.h:17,
                 from ${LINUX}/include/linux/gfp.h:6,
                 from ${LINUX}/innclude/linux/slab.h:15,
                 from ${LINUX}/linux/drivers/mmc/host/vub300.c:38:
${LINUX}/drivers/mmc/host/vub300.c: In function 'new_system_port_status':
${LINUX}/arch/sh/include/asm/string_32.h:51:42: warning: array subscript\
  80 is above array bounds of 'char[26]' [-Warray-bounds]
   : "0" (__dest), "1" (__src), "r" (__src+__n)
                                     ~~~~~^~~~

This patch fixup it

Signed-off-by: Karl Nasrallah <knnspeed@aol.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---

This is Karl's 1st pattern

 arch/sh/include/asm/string_32.h | 54 ++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 19 deletions(-)

diff --git a/arch/sh/include/asm/string_32.h b/arch/sh/include/asm/string_32.h
index 3558b1d..d35b92f 100644
--- a/arch/sh/include/asm/string_32.h
+++ b/arch/sh/include/asm/string_32.h
@@ -31,27 +31,43 @@ static inline char *strcpy(char *__dest, const char *__src)
 #define __HAVE_ARCH_STRNCPY
 static inline char *strncpy(char *__dest, const char *__src, size_t __n)
 {
-	register char *__xdest = __dest;
-	unsigned long __dummy;
+	char * retval = __dest;
+	const char * __dest_end = __dest + __n - 1;
+	register unsigned int * r0_register __asm__ ("r0");
 
+	/* size_t is always unsigned */
 	if (__n == 0)
-		return __xdest;
-
-	__asm__ __volatile__(
-		"1:\n"
-		"mov.b	@%1+, %2\n\t"
-		"mov.b	%2, @%0\n\t"
-		"cmp/eq	#0, %2\n\t"
-		"bt/s	2f\n\t"
-		" cmp/eq	%5,%1\n\t"
-		"bf/s	1b\n\t"
-		" add	#1, %0\n"
-		"2:"
-		: "=r" (__dest), "=r" (__src), "=&z" (__dummy)
-		: "0" (__dest), "1" (__src), "r" (__src+__n)
-		: "memory", "t");
-
-	return __xdest;
+		return retval;
+
+	/*
+	 * Some notes:
+	 * - cmp/eq #imm8,r0 is its own instruction
+	 * - incrementing dest and comparing to dest_end handles the size parameter in only one instruction
+	 * - mov.b R0,@Rn+ is SH2A only, but we can fill a delay slot with "add #1,%[dest]"
+	 */
+	__asm__ __volatile__ (
+		"strncpy_start:\n\t"
+		"mov.b @%[src]+,%[r0_reg]\n\t"
+		"cmp/eq #0,%[r0_reg]\n\t"
+		"bt.s strncpy_pad\n\t"
+		"cmp/eq %[dest],%[dest_end]\n\t"
+		"bt.s strncpy_end\n\t"
+		"mov.b %[r0_reg],@%[dest]\n\t"
+		"bra strncpy_start\n\t"
+		"add #1,%[dest]\n\t"
+		"strncpy_pad:\n\t"
+		"bt.s strncpy_end\n\t"
+		"mov.b %[r0_reg],@%[dest]\n\t"
+		"add #1,%[dest]\n\t"
+		"bra strncpy_pad\n\t"
+		"cmp/eq %[dest],%[dest_end]\n\t"
+		"strncpy_end:\n\t"
+		: [src] "+r" (__src), [dest] "+r" (__dest), [r0_reg] "+&z" (r0_register)
+		: [dest_end] "r" (__dest_end)
+		: "t","memory"
+		);
+
+	return retval;
 }
 
 #define __HAVE_ARCH_STRCMP
-- 
2.7.4

[PATCH] sh: fixup strncpy()

[Kuninori Morimoto]

From: Karl Nasrallah <knnspeed@aol.com>

Current SH will get below warning at strncpy()

In file included from ${LINUX}/arch/sh/include/asm/string.h:3,
                 from ${LINUX}/include/linux/string.h:20,
                 from ${LINUX}/include/linux/bitmap.h:9,
                 from ${LINUX}/include/linux/nodemask.h:95,
                 from ${LINUX}/include/linux/mmzone.h:17,
                 from ${LINUX}/include/linux/gfp.h:6,
                 from ${LINUX}/innclude/linux/slab.h:15,
                 from ${LINUX}/linux/drivers/mmc/host/vub300.c:38:
${LINUX}/drivers/mmc/host/vub300.c: In function 'new_system_port_status':
${LINUX}/arch/sh/include/asm/string_32.h:51:42: warning: array subscript\
  80 is above array bounds of 'char[26]' [-Warray-bounds]
   : "0" (__dest), "1" (__src), "r" (__src+__n)
                                     ~~~~~^~~~

This patch fixup it

Signed-off-by: Karl Nasrallah <knnspeed@aol.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---

This is Karl's 2nd pattern

 arch/sh/include/asm/string_32.h | 54 ++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 19 deletions(-)

diff --git a/arch/sh/include/asm/string_32.h b/arch/sh/include/asm/string_32.h
index 3558b1d..5a398dd 100644
--- a/arch/sh/include/asm/string_32.h
+++ b/arch/sh/include/asm/string_32.h
@@ -31,27 +31,43 @@ static inline char *strcpy(char *__dest, const char *__src)
 #define __HAVE_ARCH_STRNCPY
 static inline char *strncpy(char *__dest, const char *__src, size_t __n)
 {
-	register char *__xdest = __dest;
-	unsigned long __dummy;
+	char * retval = __dest;
+	const char * __dest_end = __dest + __n - 1;
 
+	/* size_t is always unsigned */
 	if (__n == 0)
-		return __xdest;
-
-	__asm__ __volatile__(
-		"1:\n"
-		"mov.b	@%1+, %2\n\t"
-		"mov.b	%2, @%0\n\t"
-		"cmp/eq	#0, %2\n\t"
-		"bt/s	2f\n\t"
-		" cmp/eq	%5,%1\n\t"
-		"bf/s	1b\n\t"
-		" add	#1, %0\n"
-		"2:"
-		: "=r" (__dest), "=r" (__src), "=&z" (__dummy)
-		: "0" (__dest), "1" (__src), "r" (__src+__n)
-		: "memory", "t");
-
-	return __xdest;
+		return retval;
+
+	/*
+	 * Some notes:
+	 * - cmp/eq #imm8,r0 is its own instruction
+	 * - incrementing dest and comparing to dest_end handles the size parameter in only one instruction
+	 * - mov.b R0,@Rn+ is SH2A only, but we can fill a delay slot with "add #1,%[dest]"
+	 */
+
+	__asm__ __volatile__ (
+		"strncpy_start:\n\t"
+		"mov.b @%[src]+,r0\n\t"
+		"cmp/eq #0,r0\n\t"
+		"bt.s strncpy_pad\n\t"
+		"cmp/eq %[dest],%[dest_end]\n\t"
+		"bt.s strncpy_end\n\t"
+		"mov.b r0,@%[dest]\n\t"
+		"bra strncpy_start\n\t"
+		"add #1,%[dest]\n\t"
+		"strncpy_pad:\n\t"
+		"bt.s strncpy_end\n\t"
+		"mov.b r0,@%[dest]\n\t"
+		"add #1,%[dest]\n\t"
+		"bra strncpy_pad\n\t"
+		"cmp/eq %[dest],%[dest_end]\n\t"
+		"strncpy_end:\n\t"
+		: [src] "+r" (__src), [dest] "+r" (__dest)
+		: [dest_end] "r" (__dest_end)
+		: "r0","t","memory"
+		);
+
+	return retval;
 }
 
 #define __HAVE_ARCH_STRCMP
-- 
2.7.4

Re: [PATCH] sh: use generic strncpy()

[Kuninori Morimoto]

Hi

> From: Karl Nasrallah <knnspeed@aol.com>
> 
> Current SH will get below warning at strncpy()
> 
> In file included from ${LINUX}/arch/sh/include/asm/string.h:3,
>                  from ${LINUX}/include/linux/string.h:20,
>                  from ${LINUX}/include/linux/bitmap.h:9,
>                  from ${LINUX}/include/linux/nodemask.h:95,
>                  from ${LINUX}/include/linux/mmzone.h:17,
>                  from ${LINUX}/include/linux/gfp.h:6,
>                  from ${LINUX}/innclude/linux/slab.h:15,
>                  from ${LINUX}/linux/drivers/mmc/host/vub300.c:38:
> ${LINUX}/drivers/mmc/host/vub300.c: In function 'new_system_port_status':
> ${LINUX}/arch/sh/include/asm/string_32.h:51:42: warning: array subscript\
>   80 is above array bounds of 'char[26]' [-Warray-bounds]
>    : "0" (__dest), "1" (__src), "r" (__src+__n)
>                                      ~~~~~^~~~
> 
> This patch fixup it
> 
> Signed-off-by: Karl Nasrallah <knnspeed@aol.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---

Grr, I failed Subject.
Sato-san, Rich, please let me know if you select this patch.
I can repost it with correct Subject

Thank you for your help !!
Best regards
---
Kuninori Morimoto

Re: [PATCH] sh: fixup strncpy()

[John Paul Adrian Glaubitz]

Hi Kuninori!

On 12/18/19 8:33 AM, Kuninori Morimoto wrote:
> From: Karl Nasrallah <knnspeed@aol.com>
> 
> Current SH will get below warning at strncpy()
> 
> In file included from ${LINUX}/arch/sh/include/asm/string.h:3,
>                  from ${LINUX}/include/linux/string.h:20,
>                  from ${LINUX}/include/linux/bitmap.h:9,
>                  from ${LINUX}/include/linux/nodemask.h:95,
>                  from ${LINUX}/include/linux/mmzone.h:17,
>                  from ${LINUX}/include/linux/gfp.h:6,
>                  from ${LINUX}/innclude/linux/slab.h:15,
>                  from ${LINUX}/linux/drivers/mmc/host/vub300.c:38:
> ${LINUX}/drivers/mmc/host/vub300.c: In function 'new_system_port_status':
> ${LINUX}/arch/sh/include/asm/string_32.h:51:42: warning: array subscript\
>   80 is above array bounds of 'char[26]' [-Warray-bounds]
>    : "0" (__dest), "1" (__src), "r" (__src+__n)
>                                      ~~~~~^~~~
> 
> This patch fixup it
> 
> Signed-off-by: Karl Nasrallah <knnspeed@aol.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

I very much appreciated your SH fixes as I am still maintaining a SuperH
port in Debian.

Since I don't want your fixes to fall off the table, you can ask Andrew
Morton to pick up your patches which is apparently the normal path to
choose when the original maintainers are currently not available.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] sh: fixup strncpy()

[Kuninori Morimoto]

Hi John

Thank you for mail to me

> > From: Karl Nasrallah <knnspeed@aol.com>
> > 
> > Current SH will get below warning at strncpy()
> > 
> > In file included from ${LINUX}/arch/sh/include/asm/string.h:3,
> >                  from ${LINUX}/include/linux/string.h:20,
> >                  from ${LINUX}/include/linux/bitmap.h:9,
> >                  from ${LINUX}/include/linux/nodemask.h:95,
> >                  from ${LINUX}/include/linux/mmzone.h:17,
> >                  from ${LINUX}/include/linux/gfp.h:6,
> >                  from ${LINUX}/innclude/linux/slab.h:15,
> >                  from ${LINUX}/linux/drivers/mmc/host/vub300.c:38:
> > ${LINUX}/drivers/mmc/host/vub300.c: In function 'new_system_port_status':
> > ${LINUX}/arch/sh/include/asm/string_32.h:51:42: warning: array subscript\
> >   80 is above array bounds of 'char[26]' [-Warray-bounds]
> >    : "0" (__dest), "1" (__src), "r" (__src+__n)
> >                                      ~~~~~^~~~
> > 
> > This patch fixup it
> > 
> > Signed-off-by: Karl Nasrallah <knnspeed@aol.com>
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> I very much appreciated your SH fixes as I am still maintaining a SuperH
> port in Debian.
> 
> Since I don't want your fixes to fall off the table, you can ask Andrew
> Morton to pick up your patches which is apparently the normal path to
> choose when the original maintainers are currently not available.

Actually, I had been posted it to Andrew many times [1],
but nothing happened.
And according to [2], it seems SH maintainers are still working,
thus, normal path is still them.

I had been worked for SH before, and posted many fixup patches
for many times to both Andrew and SH maintainers.
But their are still not yet reviewd/accepted.
And unfortunately, it seems I'm judged as spam from SH maintainers [2].
So I can do nothing anymore...

[1]
https://lore.kernel.org/linux-renesas-soc/87o8uyylat.wl-kuninori.morimoto.gx@renesas.com/

[2]
https://lore.kernel.org/linux-renesas-soc/656cb059-366e-06b1-0d8f-741454c472b8@landley.net/

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH] sh: fixup strncpy()

[John Paul Adrian Glaubitz]

Hi Kuninori!

On 6/1/20 1:43 AM, Kuninori Morimoto wrote:
>> Since I don't want your fixes to fall off the table, you can ask Andrew
>> Morton to pick up your patches which is apparently the normal path to
>> choose when the original maintainers are currently not available.
> 
> Actually, I had been posted it to Andrew many times [1],
> but nothing happened.

Okay, that's unfortunate.

> And according to [2], it seems SH maintainers are still working,
> thus, normal path is still them.

Yes, Rich seems to be back on track.

> I had been worked for SH before, and posted many fixup patches
> for many times to both Andrew and SH maintainers.
> But their are still not yet reviewd/accepted.
> And unfortunately, it seems I'm judged as spam from SH maintainers [2].
> So I can do nothing anymore...

I really appreciate your help and I would like to see more of your fixes
come in. Please don't be let down and let's try to get your patches
merged.

Rich has promised that he will create a new tree on his website, so I think
your patches will be merged soon.

If you have more ideas for fixes, please feel encouraged to post patches, I
will try to help you to get all of them merged. I'm very sorry that you had
to wait for such a long time.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] sh: fixup strncpy()

[John Paul Adrian Glaubitz]

Hi Kuninori!

On 6/1/20 1:43 AM, Kuninori Morimoto wrote:
>> Since I don't want your fixes to fall off the table, you can ask Andrew
>> Morton to pick up your patches which is apparently the normal path to
>> choose when the original maintainers are currently not available.
> 
> Actually, I had been posted it to Andrew many times [1],
> but nothing happened.
> And according to [2], it seems SH maintainers are still working,
> thus, normal path is still them.
> 
> I had been worked for SH before, and posted many fixup patches
> for many times to both Andrew and SH maintainers.
> But their are still not yet reviewd/accepted.
> And unfortunately, it seems I'm judged as spam from SH maintainers [2].
> So I can do nothing anymore...
I think most of your patches have been merged now [1].

Is there any patch that we missed? If yes, please repost them to the list.

Thanks,
Adrian

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3b69e8b4571125bec1f77f886174fe6cab6b9d75

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] sh: fixup strncpy()

[Kuninori Morimoto]

Hi John

> >> Since I don't want your fixes to fall off the table, you can ask Andrew
> >> Morton to pick up your patches which is apparently the normal path to
> >> choose when the original maintainers are currently not available.
> > 
> > Actually, I had been posted it to Andrew many times [1],
> > but nothing happened.
> > And according to [2], it seems SH maintainers are still working,
> > thus, normal path is still them.
> > 
> > I had been worked for SH before, and posted many fixup patches
> > for many times to both Andrew and SH maintainers.
> > But their are still not yet reviewd/accepted.
> > And unfortunately, it seems I'm judged as spam from SH maintainers [2].
> > So I can do nothing anymore...
> I think most of your patches have been merged now [1].
> 
> Is there any patch that we missed? If yes, please repost them to the list.

Nice to know.
Thanks

Thank you for your help !!

Best regards
---
Kuninori Morimoto