diff mbox series

sh: fixup strncpy()

Message ID 87zhfqhyuf.wl-kuninori.morimoto.gx@renesas.com (mailing list archive)
State New, archived
Headers show
Series sh: fixup strncpy() | expand

Commit Message

Kuninori Morimoto Dec. 18, 2019, 7:33 a.m. UTC
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(-)

Comments

John Paul Adrian Glaubitz May 31, 2020, 9:43 a.m. UTC | #1
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
Kuninori Morimoto May 31, 2020, 11:43 p.m. UTC | #2
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
John Paul Adrian Glaubitz June 1, 2020, 8:15 a.m. UTC | #3
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 June 7, 2020, 12:34 p.m. UTC | #4
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
Kuninori Morimoto June 8, 2020, 12:57 a.m. UTC | #5
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
diff mbox series

Patch

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