Re: [PATCH] provide out-of-line strcat() for m68k

Previous thread: [PATCH] caiaq endianness fix by Al Viro on Tuesday, May 20, 2008 - 6:11 pm. (1 message)

Next thread: [PATCH] HTC_EGPIO is ARM-only by Al Viro on Tuesday, May 20, 2008 - 6:12 pm. (1 message)
From: Al Viro
Date: Tuesday, May 20, 2008 - 6:12 pm

Content-Length: 788
Lines: 30

Whether we sidestep it in init/main.c or not, such situations
will arise again; compiler does generate calls of strcat()
on optimizations, so we really ought to have an out-of-line
version...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/m68k/lib/string.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/m68k/lib/string.c b/arch/m68k/lib/string.c
index 891e134..4253f87 100644
--- a/arch/m68k/lib/string.c
+++ b/arch/m68k/lib/string.c
@@ -15,6 +15,12 @@ char *strcpy(char *dest, const char *src)
 }
 EXPORT_SYMBOL(strcpy);
 
+char *strcat(char *dest, const char *src)
+{
+	return __kernel_strcpy(dest + __kernel_strlen(dest), src);
+}
+EXPORT_SYMBOL(strcat);
+
 void *memset(void *s, int c, size_t count)
 {
 	void *xs = s;
-- 
1.5.3.GIT


--

From: Roman Zippel
Date: Tuesday, May 20, 2008 - 8:34 pm

Hi,


It actually was strlen that was generated and not strcat.

bye, Roman
--

From: Al Viro
Date: Tuesday, May 20, 2008 - 10:30 pm

Here it replaced strncat() with call of strcat() (gcc 4.0.1, FWIW).
And yes, I can show you init/main.s with
	jbsr strcat	|
in it generated on kernel in b0rken range...
--

From: Roman Zippel
Date: Wednesday, May 21, 2008 - 3:53 am

Hi,


Please use a more recent compiler, 4.0 created too many problems on m68k, 
which we only got under control with 4.1, so at least on m68k 4.0 is not 
really supported.

bye, Roman
--

From: Al Viro
Date: Wednesday, May 21, 2008 - 4:09 am

Then it's worth mentioning in Documentation/Changes, IMO...  Anyway,
updating m68k toolchain is not a problem; I'll get around to it tonight.
I still think that out-of-line implementation is a good idea, if nothing
else it would prevent future crap of the same kind if some later version
decides that strlen(a) + strlen(b) can be proven to be less than size
argument of strncat(), etc.

Technically we _are_ in nasal daemon country with redefining str*, unless
we pass -ffreestanding; m68k doesn't, so we can't guarantee that new stuff
of that kind won't crop up.  IOW, it might be a good policy to have fallback
implementations of potentially affected primitives...
--

From: Adrian Bunk
Date: Wednesday, May 21, 2008 - 12:52 am

Can we try to get this sorted out properly instead of constantly 
fiddling with it?

Currently we use -ffreestanding on some architectures and fix breakages 
on the other architectures when they arise.

The options I see for getting this fixed properly are:
- use -ffreestanding on all architectures or
- don't use -ffreestanding on any architecture and move the string
  functions (and perhaps other functions if required) out-of-line

I'd prefer the first option, but I know that not everyone agrees with me.

No matter which option we choose, if we get an agreement on this one
I can send patches for it.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--

From: Roman Zippel
Date: Wednesday, May 21, 2008 - 4:44 am

Hi,


This won't help completely unless you also clean up all archs to use the 
same mappings to the builtin functions.

The main problem I had with -ffreestanding is that it's awkward to map a 
library function to the builtin function and also provide the fallback 
from lib/string.c.
If you look at asm-m68k/string.h I once tried this with the mem* functions 
and I still have the duplicated memcmp in arch/m68k/lib/string.c.
(You could argue that it would be easier to just remove the define for 
memcmp in this specific case, but I'm interested in the general case.)

bye, Roman
--

Previous thread: [PATCH] caiaq endianness fix by Al Viro on Tuesday, May 20, 2008 - 6:11 pm. (1 message)

Next thread: [PATCH] HTC_EGPIO is ARM-only by Al Viro on Tuesday, May 20, 2008 - 6:12 pm. (1 message)