This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH] Simplify strncat.
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: libc-alpha at sourceware dot org
- Date: Tue, 16 Dec 2014 21:24:38 +0100
- Subject: [PATCH] Simplify strncat.
- Authentication-results: sourceware.org; auth=none
Hi, as in another thread found warning in default strncat implementation
I looked to code and best way looks to replace loop by
memcpy (dest, src, strnlen (src, n));
Its because code look misoptimized, a loop does byte-by-byte copy
instead copying 4 bytes at once like
for (i = 0; i < n; i += 4)
{
uint32 srcm = *((uint32_t *)(src + i));
if (has_zero (srcm)) /* use trick from strlen */
break;
*((uint32_t *)(dst + i)) = *srcm;
}
where I for simplicity ommited aligning dest and simulating unaligned
read when architecture does not allow it.
As even generic memcpy and strnlen do these trick code will be lot
faster unless you do most of calls to append 1-4 bytes.
As strncat should not be used for performance sensitive task I decided
to keep that simple. Also on my machine its almost never used. Only
application that I could find is google-chrome which called strnlen 44
times total with following lenghts.
dest src n
56 198 198
254 75 75
329 94 94
423 24 24
56 198 198
254 75 75
329 94 94
423 24 24
56 198 198
254 75 75
329 94 94
423 24 24
Tested on x64 with modified test, OK to commit?
* string/strncat.c (STRNCAT): Simplify implementation.
diff --git a/string/strncat.c b/string/strncat.c
index 6d29114..e130a1f 100644
--- a/string/strncat.c
+++ b/string/strncat.c
@@ -29,52 +29,15 @@
char *
STRNCAT (char *s1, const char *s2, size_t n)
{
- char c;
char *s = s1;
/* Find the end of S1. */
s1 += strlen (s1);
- /* Make S1 point before next character, so we can increment
- it while memory is read (wins on pipelined cpus). */
- s1 -= 1;
+ size_t ss = __strnlen (s2, n);
- if (n >= 4)
- {
- size_t n4 = n >> 2;
- do
- {
- c = *s2++;
- *++s1 = c;
- if (c == '\0')
- return s;
- c = *s2++;
- *++s1 = c;
- if (c == '\0')
- return s;
- c = *s2++;
- *++s1 = c;
- if (c == '\0')
- return s;
- c = *s2++;
- *++s1 = c;
- if (c == '\0')
- return s;
- } while (--n4 > 0);
- n &= 3;
- }
-
- while (n > 0)
- {
- c = *s2++;
- *++s1 = c;
- if (c == '\0')
- return s;
- n--;
- }
-
- if (c != '\0')
- *++s1 = '\0';
+ memcpy (s1, s2, ss);
+ s1[ss] = '\000';
return s;
}