[PATCH] String: expand to word size && refactor || refactor

[PATCH] String: expand to word size && refactor || refactor

[Nathan McSween]

Reworking of last expanding / refactoring of string fns, logic should be
reviewed as I had issues syncing changes across revisions.

Nathan McSween (1):
  String: expand to word size && refactor || refactor

 src/string/memccpy.c   | 50 ++++++++++++++++++++++++++----------------------
 src/string/memchr.c    | 42 +++++++++++++++++++++++-----------------
 src/string/memcmp.c    | 33 ++++++++++++++++++++++++++++----
 src/string/memcpy.c    | 44 +++++++++++++++++++++---------------------
 src/string/memrchr.c   | 30 +++++++++++++++++++++++++----
 src/string/memset.c    | 37 ++++++++++++++++++++---------------
 src/string/stpcpy.c    | 32 +++++++++++++++++--------------
 src/string/stpncpy.c   | 38 +++++++++++++++++++-----------------
 src/string/strcat.c    |  7 ++++---
 src/string/strchrnul.c | 26 ++++++++++++++-----------
 src/string/strcmp.c    | 30 ++++++++++++++++++++++++++++-
 src/string/strcpy.c    |  7 -------
 src/string/strlcpy.c   | 52 +++++++++++++++++++++++++++++---------------------
 13 files changed, 268 insertions(+), 160 deletions(-)

-- 
1.8.1.2

[PATCH] String: expand to word size && refactor || refactor

[Nathan McSween]

---
 src/string/memccpy.c   | 50 ++++++++++++++++++++++++++----------------------
 src/string/memchr.c    | 42 +++++++++++++++++++++++-----------------
 src/string/memcmp.c    | 33 ++++++++++++++++++++++++++++----
 src/string/memcpy.c    | 44 +++++++++++++++++++++---------------------
 src/string/memrchr.c   | 30 +++++++++++++++++++++++++----
 src/string/memset.c    | 37 ++++++++++++++++++++---------------
 src/string/stpcpy.c    | 32 +++++++++++++++++--------------
 src/string/stpncpy.c   | 38 +++++++++++++++++++-----------------
 src/string/strcat.c    |  7 ++++---
 src/string/strchrnul.c | 26 ++++++++++++++-----------
 src/string/strcmp.c    | 30 ++++++++++++++++++++++++++++-
 src/string/strcpy.c    |  7 -------
 src/string/strlcpy.c   | 52 +++++++++++++++++++++++++++++---------------------
 13 files changed, 268 insertions(+), 160 deletions(-)

diff --git a/src/string/memccpy.c b/src/string/memccpy.c
index b85009c..f032030 100644
--- a/src/string/memccpy.c
+++ b/src/string/memccpy.c
@@ -1,32 +1,36 @@
 #include <string.h>
-#include <stdlib.h>
 #include <stdint.h>
-#include <limits.h>
 
-#define ALIGN (sizeof(size_t)-1)
-#define ONES ((size_t)-1/UCHAR_MAX)
-#define HIGHS (ONES * (UCHAR_MAX/2+1))
-#define HASZERO(x) ((x)-ONES & ~(x) & HIGHS)
+#define HIGHS(x) ((x) - ((x)*0-1) / 255 * 128)
+#define LOWS(x) (((x)*0-1) / 255)
+#define has_char(x, c) ((x) - LOWS(x) & ~(x) & HIGHS(x) ^ LOWS(x) * (c))
 
-void *memccpy(void *restrict dest, const void *restrict src, int c, size_t n)
+void *memccpy(void *restrict d, const void *restrict s, int c, size_t n)
 {
-	unsigned char *d = dest;
-	const unsigned char *s = src;
-	size_t *wd, k;
+	unsigned char *cd = (unsigned char *)d;
+	const unsigned char *cs = (const unsigned char *)s;
+	size_t *wd;
 	const size_t *ws;
 
 	c = (unsigned char)c;
-	if (((uintptr_t)s & ALIGN) == ((uintptr_t)d & ALIGN)) {
-		for (; ((uintptr_t)s & ALIGN) && n && (*d=*s)!=c; n--, s++, d++);
-		if ((uintptr_t)s & ALIGN) goto tail;
-		k = ONES * c;
-		wd=(void *)d; ws=(const void *)s;
-		for (; n>=sizeof(size_t) && !HASZERO(*ws^k);
-		       n-=sizeof(size_t), ws++, wd++) *wd = *ws;
-		d=(void *)wd; s=(const void *)ws;
-	}
-	for (; n && (*d=*s)!=c; n--, s++, d++);
-tail:
-	if (*s==c) return d+1;
-	return 0;
+
+	if ((uintptr_t)s % sizeof(size_t) != (uintptr_t)d % sizeof(size_t)
+	    || n < sizeof(size_t))
+		goto bytewise;
+
+	for (; (uintptr_t)s % sizeof(size_t); *cd = *cs, cd++, cs++)
+		if (*cs == c) return cd + 1;
+
+	for (wd = (size_t *)d, ws = (const size_t *)s
+	     ; !has_char(*ws, c) && n >= sizeof(size_t)
+	     ; ws++, wd++, *wd = *ws);
+
+	cd = (unsigned char *)wd;
+	cs = (const unsigned char *)ws;
+
+bytewise:
+	for (; *cs != c; *cd = *cs, cs++, cd++, n--)
+		if (!n) return NULL;
+
+	return cd + 1;
 }
diff --git a/src/string/memchr.c b/src/string/memchr.c
index a0472f7..9a69a85 100644
--- a/src/string/memchr.c
+++ b/src/string/memchr.c
@@ -1,24 +1,32 @@
-#include <string.h>
-#include <stdlib.h>
+#include <stddef.h>
 #include <stdint.h>
-#include <limits.h>
+#include <string.h>
 
-#define SS (sizeof(size_t))
-#define ALIGN (sizeof(size_t)-1)
-#define ONES ((size_t)-1/UCHAR_MAX)
-#define HIGHS (ONES * (UCHAR_MAX/2+1))
-#define HASZERO(x) ((x)-ONES & ~(x) & HIGHS)
+#define HIGHS(x) ((x) - ((x)*0-1) / 255 * 128)
+#define LOWS(x) (((x)*0-1) / 255)
+#define has_char(x, c) ((x) - LOWS(x) & ~(x) & HIGHS(x) ^ LOWS(x) * (c))
 
-void *memchr(const void *src, int c, size_t n)
+void *memchr(const void *s, int c, size_t n)
 {
-	const unsigned char *s = src;
+	const unsigned char *cs = (const unsigned char *)s;
+	const size_t *ws;
+
 	c = (unsigned char)c;
-	for (; ((uintptr_t)s & ALIGN) && n && *s != c; s++, n--);
-	if (n && *s != c) {
-		const size_t *w;
-		size_t k = ONES * c;
-		for (w = (const void *)s; n>=SS && !HASZERO(*w^k); w++, n-=SS);
-		for (s = (const void *)w; n && *s != c; s++, n--);
+
+	if (n < sizeof(size_t)) goto bytewise;
+
+	for (; (uintptr_t)cs % sizeof(size_t); cs++, n--) {
+		if (*cs == c) return (void *)cs;
 	}
-	return n ? (void *)s : 0;
+
+	for (ws = (const size_t *)cs
+	     ; !has_char(*ws, c) && n >= sizeof(size_t)
+	     ; ws++, n -= sizeof(size_t));
+	cs = (const unsigned char *)ws;
+
+bytewise:
+	for (; *cs != c; cs++, n--)
+		if (!n) return NULL;
+
+	return (void *)cs;
 }
diff --git a/src/string/memcmp.c b/src/string/memcmp.c
index bdbce9f..81b1e80 100644
--- a/src/string/memcmp.c
+++ b/src/string/memcmp.c
@@ -1,8 +1,33 @@
+#include <stddef.h>
+#include <stdint.h>
 #include <string.h>
 
-int memcmp(const void *vl, const void *vr, size_t n)
+int memcmp(const void *s, const void *c, size_t n)
 {
-	const unsigned char *l=vl, *r=vr;
-	for (; n && *l == *r; n--, l++, r++);
-	return n ? *l-*r : 0;
+	const unsigned char *cs = (const unsigned char *)s;
+	const unsigned char *cc = (const unsigned char *)c;
+	const size_t *ws;
+	const size_t *wc;
+
+	if ((uintptr_t)cs % sizeof(size_t) != (uintptr_t)cc % sizeof(size_t)
+	    || n < sizeof(size_t)) {
+		goto bytewise;
+	}
+
+	for (; (uintptr_t)cs % sizeof(size_t); cs++, cc++, n--)
+		if (*cs == *cc) return *cs - *cc;
+
+	for (ws = (const size_t *)cs, wc = (const size_t *)cc
+	     ; *ws == *wc && n >= sizeof(size_t)
+	     ; ws++, wc++, n -= sizeof(size_t));
+	cs = (const unsigned char *)ws;
+	cc = (const unsigned char *)wc;
+
+bytewise:
+	for(; *cs == *cc; cs++, cc++, n--) {
+		if (!n) return 0;
+	}
+
+	return *cs - *cc;
 }
+
diff --git a/src/string/memcpy.c b/src/string/memcpy.c
index 8e98302..c34185c 100644
--- a/src/string/memcpy.c
+++ b/src/string/memcpy.c
@@ -1,29 +1,29 @@
-#include <string.h>
-#include <stdlib.h>
+#include <stddef.h>
 #include <stdint.h>
+#include <string.h>
 
-#define SS (sizeof(size_t))
-#define ALIGN (sizeof(size_t)-1)
-#define ONES ((size_t)-1/UCHAR_MAX)
-
-void *memcpy(void *restrict dest, const void *restrict src, size_t n)
+void *memcpy(void *restrict d, const void *restrict s, size_t n)
 {
-	unsigned char *d = dest;
-	const unsigned char *s = src;
+	unsigned char *cd = (unsigned char *)d;
+	const unsigned char *cs = (const unsigned char *)s;
+	size_t *wd;
+	const size_t *ws;
 
-	if (((uintptr_t)d & ALIGN) != ((uintptr_t)s & ALIGN))
-		goto misaligned;
+	if ((uintptr_t)cd % sizeof(size_t) != (uintptr_t)cs % sizeof(size_t)
+	    || n < sizeof(size_t)) {
+		goto bytewise;
+	}
 
-	for (; ((uintptr_t)d & ALIGN) && n; n--) *d++ = *s++;
-	if (n) {
-		size_t *wd = (void *)d;
-		const size_t *ws = (const void *)s;
+	for (; (uintptr_t)cd % sizeof(size_t); *cd++ = *cs++, n--);
 
-		for (; n>=SS; n-=SS) *wd++ = *ws++;
-		d = (void *)wd;
-		s = (const void *)ws;
-misaligned:
-		for (; n; n--) *d++ = *s++;
-	}
-	return dest;
+	for (wd = (size_t *)cd, ws = (const size_t *)cs
+	     ; n >= sizeof(size_t)
+	     ; n -= sizeof(size_t), *wd++ = *ws++);
+	cd = (unsigned char *)wd;
+	cs = (const unsigned char *)ws;
+
+bytewise:
+	for (; n; *cd++ = *cs++, n--);
+
+	return d;
 }
diff --git a/src/string/memrchr.c b/src/string/memrchr.c
index a78e9d6..e9bb838 100644
--- a/src/string/memrchr.c
+++ b/src/string/memrchr.c
@@ -1,12 +1,34 @@
+#include <stddef.h>
+#include <stdint.h>
 #include <string.h>
 #include "libc.h"
 
-void *__memrchr(const void *m, int c, size_t n)
+#define HIGHS(x) ((x) - ((x)*0-1) / 255 * 128)
+#define LOWS(x) (((x)*0-1) / 255)
+#define has_char(x, c) ((x) - LOWS(x) & ~(x) & HIGHS(x) ^ LOWS(x) * (c))
+
+void *__memrchr(const void *s, int c, size_t n)
 {
-	const unsigned char *s = m;
+	const unsigned char *cs = s;
+	const size_t *ws;
+
 	c = (unsigned char)c;
-	while (n--) if (s[n]==c) return (void *)(s+n);
-	return 0;
+
+	if (n < sizeof(size_t)) goto bytewise;
+
+	for (; (uintptr_t)s % sizeof(size_t); cs++, n--)
+		if (cs[n] == c) return (void *)(cs + n);
+
+	for (ws = (const size_t *)cs + n
+	     ; !has_char(*ws, c) && n >= sizeof(size_t)
+	     ; ws--, n -= sizeof(size_t));
+	cs = (const unsigned char *)ws;
+
+bytewise:
+	for (; n; n--)
+		if (cs[n] == c) return (void *)(cs + n);
+
+	return NULL;
 }
 
 weak_alias(__memrchr, memrchr);
diff --git a/src/string/memset.c b/src/string/memset.c
index 20e47c4..774829d 100644
--- a/src/string/memset.c
+++ b/src/string/memset.c
@@ -1,21 +1,28 @@
-#include <string.h>
-#include <stdlib.h>
+#include <stddef.h>
 #include <stdint.h>
-#include <limits.h>
+#include <string.h>
 
-#define SS (sizeof(size_t))
-#define ALIGN (sizeof(size_t)-1)
-#define ONES ((size_t)-1/UCHAR_MAX)
+#define LOWS(x) (((x)*0-1) / 255)
 
-void *memset(void *dest, int c, size_t n)
+void *memset(void *d, int c, size_t n)
 {
-	unsigned char *s = dest;
+	unsigned char *cd = (unsigned char *)d;
+	const size_t wc = LOWS(wc) * (unsigned char)c;
+	size_t *wd;
+
 	c = (unsigned char)c;
-	for (; ((uintptr_t)s & ALIGN) && n; n--) *s++ = c;
-	if (n) {
-		size_t *w, k = ONES * c;
-		for (w = (void *)s; n>=SS; n-=SS, w++) *w = k;
-		for (s = (void *)w; n; n--, s++) *s = c;
-	}
-	return dest;
+
+	if (n < sizeof(size_t)) goto bytewise;
+
+	for (; (uintptr_t)d % sizeof(size_t); *cd++ = c, n--);
+
+	for (wd = (size_t *)cd
+	     ; n >= sizeof(size_t)
+	     ; *wd++ = wc, n -= sizeof(size_t));
+	cd = (unsigned char *)wd;
+
+bytewise:
+	for (; n; *cd++ = c, n--);
+
+	return d;
 }
diff --git a/src/string/stpcpy.c b/src/string/stpcpy.c
index feb9eb8..6608c38 100644
--- a/src/string/stpcpy.c
+++ b/src/string/stpcpy.c
@@ -1,27 +1,31 @@
+#include <stddef.h>
 #include <string.h>
-#include <stdlib.h>
 #include <stdint.h>
-#include <limits.h>
 #include "libc.h"
 
-#define ALIGN (sizeof(size_t))
-#define ONES ((size_t)-1/UCHAR_MAX)
-#define HIGHS (ONES * (UCHAR_MAX/2+1))
-#define HASZERO(x) ((x)-ONES & ~(x) & HIGHS)
+#define HIGHS(x) ((x) - ((x)*0-1) / 255 * 128)
+#define LOWS(x) (((x)*0-1) / 255)
+#define has_zero(x) ((x) - LOWS(x) & ~(x) & HIGHS(x))
 
 char *__stpcpy(char *restrict d, const char *restrict s)
 {
 	size_t *wd;
 	const size_t *ws;
 
-	if ((uintptr_t)s % ALIGN == (uintptr_t)d % ALIGN) {
-		for (; (uintptr_t)s % ALIGN; s++, d++)
-			if (!(*d=*s)) return d;
-		wd=(void *)d; ws=(const void *)s;
-		for (; !HASZERO(*ws); *wd++ = *ws++);
-		d=(void *)wd; s=(const void *)ws;
-	}
-	for (; (*d=*s); s++, d++);
+	if ((uintptr_t)s % sizeof(size_t) != (uintptr_t)d % sizeof(size_t))
+		goto bytewise;
+
+	for (; (uintptr_t)s % sizeof(size_t); s++, d++)
+		if (!(*d = *s)) return d;
+
+	for (wd = (size_t *)d, ws = (const size_t *)s
+	     ; !has_zero(*ws); wd++, ws++, *wd = *ws);
+
+	d = (char *)wd;
+	s = (const char *)ws;
+
+bytewise:
+	for (; (*d = *s); d++, s++);
 
 	return d;
 }
diff --git a/src/string/stpncpy.c b/src/string/stpncpy.c
index 0a2c2a9..c38d98d 100644
--- a/src/string/stpncpy.c
+++ b/src/string/stpncpy.c
@@ -1,32 +1,36 @@
+#include <stddef.h>
 #include <string.h>
-#include <stdlib.h>
 #include <stdint.h>
-#include <limits.h>
 #include "libc.h"
 
-#define ALIGN (sizeof(size_t)-1)
-#define ONES ((size_t)-1/UCHAR_MAX)
-#define HIGHS (ONES * (UCHAR_MAX/2+1))
-#define HASZERO(x) ((x)-ONES & ~(x) & HIGHS)
+#define HIGHS(x) ((x) - ((x)*0-1) / 255 * 128)
+#define LOWS(x) (((x)*0-1) / 255)
+#define has_zero(x) ((x) - LOWS(x) & ~(x) & HIGHS(x))
 
 char *__stpncpy(char *restrict d, const char *restrict s, size_t n)
 {
 	size_t *wd;
 	const size_t *ws;
 
-	if (((uintptr_t)s & ALIGN) == ((uintptr_t)d & ALIGN)) {
-		for (; ((uintptr_t)s & ALIGN) && n && (*d=*s); n--, s++, d++);
-		if (!n || !*s) goto tail;
-		wd=(void *)d; ws=(const void *)s;
-		for (; n>=sizeof(size_t) && !HASZERO(*ws);
-		       n-=sizeof(size_t), ws++, wd++) *wd = *ws;
-		d=(void *)wd; s=(const void *)ws;
-	}
-	for (; n && (*d=*s); n--, s++, d++);
-tail:
+	if ((uintptr_t)s % sizeof(size_t) == (uintptr_t)d % sizeof(size_t))
+		goto bytewise;
+
+	for (; (uintptr_t)s % sizeof(size_t) && (*d = *s); d++, s++, n--)
+		if (!n || !*s) goto terminate;
+
+	for (wd = (size_t *)d, ws = (const size_t *)s
+	     ; !has_zero(*ws) && n >= sizeof(size_t)
+	     ; n -= sizeof(size_t), ws++, wd++, *wd = *ws);
+
+	d = (char *)wd;
+	s = (const char *)ws;
+
+bytewise:
+	for (; (*d = *s) && n; d++, s++, n--);
+terminate:
 	memset(d, 0, n);
+
 	return d;
 }
 
 weak_alias(__stpncpy, stpncpy);
-
diff --git a/src/string/strcat.c b/src/string/strcat.c
index 33f749b..0919793 100644
--- a/src/string/strcat.c
+++ b/src/string/strcat.c
@@ -1,7 +1,8 @@
 #include <string.h>
 
-char *strcat(char *restrict dest, const char *restrict src)
+char *strcat(char *restrict d, const char *restrict s)
 {
-	strcpy(dest + strlen(dest), src);
-	return dest;
+	strcpy(d + strlen(d), s);
+
+	return d;
 }
diff --git a/src/string/strchrnul.c b/src/string/strchrnul.c
index ceae4d4..cebd063 100644
--- a/src/string/strchrnul.c
+++ b/src/string/strchrnul.c
@@ -1,26 +1,30 @@
+#include <stddef.h>
 #include <string.h>
-#include <stdlib.h>
 #include <stdint.h>
-#include <limits.h>
 #include "libc.h"
 
-#define ALIGN (sizeof(size_t))
-#define ONES ((size_t)-1/UCHAR_MAX)
-#define HIGHS (ONES * (UCHAR_MAX/2+1))
-#define HASZERO(x) ((x)-ONES & ~(x) & HIGHS)
+#define HIGHS(x) ((x) - ((x)*0-1) / 255 * 128)
+#define LOWS(x) (((x)*0-1) / 255)
+#define has_zero(x) ((x) - LOWS(x) & ~(x) & HIGHS(x))
+#define has_char(x, c) ((x) - LOWS(x) & ~(x) & HIGHS(x) ^ LOWS(x) * (c))
 
 char *__strchrnul(const char *s, int c)
 {
-	size_t *w, k;
+	const size_t *ws;
 
 	c = (unsigned char)c;
+
 	if (!c) return (char *)s + strlen(s);
 
-	for (; (uintptr_t)s % ALIGN; s++)
+	for (; (uintptr_t)s % sizeof(size_t); s++)
 		if (!*s || *(unsigned char *)s == c) return (char *)s;
-	k = ONES * c;
-	for (w = (void *)s; !HASZERO(*w) && !HASZERO(*w^k); w++);
-	for (s = (void *)w; *s && *(unsigned char *)s != c; s++);
+
+	for (ws = (const size_t *)s; !has_zero(*ws) && !has_char(*ws, c); ws++);
+
+	s = (const char *)ws;
+
+	for (; *s && *(unsigned char *)s != c; s++);
+
 	return (char *)s;
 }
 
diff --git a/src/string/strcmp.c b/src/string/strcmp.c
index 91eb740..98b5e58 100644
--- a/src/string/strcmp.c
+++ b/src/string/strcmp.c
@@ -1,7 +1,35 @@
+#include <stddef.h>
+#include <stdint.h>
 #include <string.h>
 
+#define HIGHS(x) ((x) - ((x)*0-1) / 255 * 128)
+#define LOWS(x) (((x)*0-1) / 255)
+#define has_zero(x) ((x) - LOWS(x) & ~(x) & HIGHS(x))
+
+#undef strcmp
 int strcmp(const char *l, const char *r)
 {
-	for (; *l==*r && *l && *r; l++, r++);
+	const size_t *wl;
+	const size_t *wr;
+
+	if ((uintptr_t)l % sizeof(size_t) != (uintptr_t)r % sizeof(size_t))
+		goto bytewise;
+
+	for (; (uintptr_t)l % sizeof(size_t); l++, r++) {
+		if (*l != *r || !*l || !*r) {
+			return *(unsigned char *)l
+			       - *(unsigned char *)r;
+		}
+	}
+
+	for (wl = (const size_t *)l, wr = (const size_t *)r
+	     ; has_zero(*wl) || has_zero(*wr) && *wl == *wr
+	     ; wl++, wr++);
+	l = (const char *)wl;
+	r = (const char *)wr;
+
+bytewise:
+	for(; *l == *r && *l && *r; l++, r++);
+
 	return *(unsigned char *)l - *(unsigned char *)r;
 }
diff --git a/src/string/strcpy.c b/src/string/strcpy.c
index f7e3ba3..2883e93 100644
--- a/src/string/strcpy.c
+++ b/src/string/strcpy.c
@@ -4,13 +4,6 @@ char *__stpcpy(char *, const char *);
 
 char *strcpy(char *restrict dest, const char *restrict src)
 {
-#if 1
 	__stpcpy(dest, src);
 	return dest;
-#else
-	const unsigned char *s = src;
-	unsigned char *d = dest;
-	while ((*d++ = *s++));
-	return dest;
-#endif
 }
diff --git a/src/string/strlcpy.c b/src/string/strlcpy.c
index 4d3ff92..42b63e1 100644
--- a/src/string/strlcpy.c
+++ b/src/string/strlcpy.c
@@ -1,32 +1,40 @@
-#include <string.h>
-#include <stdlib.h>
+#include <stddef.h>
 #include <stdint.h>
-#include <limits.h>
-#include "libc.h"
+#include <string.h>
 
-#define ALIGN (sizeof(size_t)-1)
-#define ONES ((size_t)-1/UCHAR_MAX)
-#define HIGHS (ONES * (UCHAR_MAX/2+1))
-#define HASZERO(x) ((x)-ONES & ~(x) & HIGHS)
+#define HIGHS(x) ((x) - ((x)*0-1) / 255 * 128)
+#define LOWS(x) (((x)*0-1) / 255)
+#define has_zero(x) ((x) - LOWS(x) & ~(x) & HIGHS(x))
 
 size_t strlcpy(char *d, const char *s, size_t n)
 {
-	char *d0 = d;
+	char *ds = d;
 	size_t *wd;
 	const size_t *ws;
 
-	if (!n--) goto finish;
-	if (((uintptr_t)s & ALIGN) == ((uintptr_t)d & ALIGN)) {
-		for (; ((uintptr_t)s & ALIGN) && n && (*d=*s); n--, s++, d++);
-		if (n && *s) {
-			wd=(void *)d; ws=(const void *)s;
-			for (; n>=sizeof(size_t) && !HASZERO(*ws);
-			       n-=sizeof(size_t), ws++, wd++) *wd = *ws;
-			d=(void *)wd; s=(const void *)ws;
-		}
+	if (!n--) goto ret;
+
+	if ((uintptr_t)d % sizeof(size_t) != (uintptr_t)s % sizeof(size_t)
+	    || n < sizeof(size_t)) {
+		goto bytewise;
 	}
-	for (; n && (*d=*s); n--, s++, d++);
-	*d = 0;
-finish:
-	return d-d0 + strlen(s);
+
+	for (; (uintptr_t)s % sizeof(size_t); *d++ = *s++, n--)
+		if (!*s) goto terminate;
+
+	for (wd = (size_t *)d, ws = (const size_t *)s
+	     ; !has_zero(*ws) && n >= sizeof(size_t)
+	     ; *wd++ = *ws++, n -= sizeof(size_t))
+
+	d = (char *)wd;
+	s = (const char *)ws;
+
+bytewise:
+	for (; (*d = *s) && n; d++, s++, n--);
+
+terminate:
+	*d = '\0';
+
+ret:
+	return (d - ds + strlen(s));
 }
-- 
1.8.1.2

Re: [PATCH] String: expand to word size && refactor || refactor

[Szabolcs Nagy]

* Nathan McSween <nwmcsween@gmail.com> [2013-04-06 20:38:16 +0000]:
> diff --git a/src/string/memccpy.c b/src/string/memccpy.c
> index b85009c..f032030 100644
> --- a/src/string/memccpy.c
> +++ b/src/string/memccpy.c
> @@ -1,32 +1,36 @@
>  #include <string.h>
> -#include <stdlib.h>
>  #include <stdint.h>
> -#include <limits.h>
>  
> -#define ALIGN (sizeof(size_t)-1)
> -#define ONES ((size_t)-1/UCHAR_MAX)
> -#define HIGHS (ONES * (UCHAR_MAX/2+1))
> -#define HASZERO(x) ((x)-ONES & ~(x) & HIGHS)
> +#define HIGHS(x) ((x) - ((x)*0-1) / 255 * 128)
> +#define LOWS(x) (((x)*0-1) / 255)
> +#define has_char(x, c) ((x) - LOWS(x) & ~(x) & HIGHS(x) ^ LOWS(x) * (c))

the meaning of the new HIGHS is unclear

the original macros are easier to read, HIGHS is probably not
needed and i think size_t can be hardcoded, so

#define ONES ((size_t)-1/255)
#define HASZERO(x) ((x)-ONES & ~(x) & ONES*128)

a has_char can be useful for clarity (but the original code
made sure ones*c is only calculated once) and then i'd use

#define HASCHAR(x,c) HASZERO((x) ^ ONES*(unsigned char)(c))

> -void *memccpy(void *restrict dest, const void *restrict src, int c, size_t n)
> +void *memccpy(void *restrict d, const void *restrict s, int c, size_t n)
>  {
> -	unsigned char *d = dest;
> -	const unsigned char *s = src;
> -	size_t *wd, k;
> +	unsigned char *cd = (unsigned char *)d;
> +	const unsigned char *cs = (const unsigned char *)s;
> +	size_t *wd;
>  	const size_t *ws;
>  
>  	c = (unsigned char)c;
> -	if (((uintptr_t)s & ALIGN) == ((uintptr_t)d & ALIGN)) {
> -		for (; ((uintptr_t)s & ALIGN) && n && (*d=*s)!=c; n--, s++, d++);
> -		if ((uintptr_t)s & ALIGN) goto tail;
> -		k = ONES * c;
> -		wd=(void *)d; ws=(const void *)s;
> -		for (; n>=sizeof(size_t) && !HASZERO(*ws^k);
> -		       n-=sizeof(size_t), ws++, wd++) *wd = *ws;
> -		d=(void *)wd; s=(const void *)ws;
> -	}
> -	for (; n && (*d=*s)!=c; n--, s++, d++);
> -tail:
> -	if (*s==c) return d+1;
> -	return 0;
> +
> +	if ((uintptr_t)s % sizeof(size_t) != (uintptr_t)d % sizeof(size_t)
> +	    || n < sizeof(size_t))
> +		goto bytewise;
> +
> +	for (; ; *cd = *cs, cd++, cs++)
> +		if (*cs == c) return cd + 1;

if ((*cd++ = *cs++) == c) return cd; is more idiomatic

with your code
the c at the end is not copied, this is a bug
the n limit is not accounted for, this is a bug
the code below is never reached, this is another bug

> +	for (wd = (size_t *)d, ws = (const size_t *)s
> +	     ; !has_char(*ws, c) && n >= sizeof(size_t)
> +	     ; ws++, wd++, *wd = *ws);
> +
> +	cd = (unsigned char *)wd;
> +	cs = (const unsigned char *)ws;
> +
> +bytewise:
> +	for (; *cs != c; *cd = *cs, cs++, cd++, n--)
> +		if (!n) return NULL;
> +
> +	return cd + 1;
>  }

i think you should test and benchmark before submitting
such code

Re: [PATCH] String: expand to word size && refactor || refactor

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 518 bytes --]

Hello

Am Sonntag, den 07.04.2013, 11:23 +0200 schrieb Szabolcs Nagy:
> #define ONES ((size_t)-1/255)

just a nitpick, wouldn't

#define ONES (SIZE_MAX/255)

be clearer

Jens

-- 
:: INRIA Nancy Grand Est :: http://www.loria.fr/~gustedt/   ::
:: AlGorille ::::::::::::::: office Nancy : +33 383593090   ::
:: ICube :::::::::::::: office Strasbourg : +33 368854536   ::
:: ::::::::::::::::::::::::::: gsm France : +33 651400183   ::
:: :::::::::::::::::::: gsm international : +49 15737185122 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] String: expand to word size && refactor || refactor

[Szabolcs Nagy]

* Jens Gustedt <jens.gustedt@inria.fr> [2013-04-07 11:38:48 +0200]:
> Am Sonntag, den 07.04.2013, 11:23 +0200 schrieb Szabolcs Nagy:
> > #define ONES ((size_t)-1/255)
> 
> just a nitpick, wouldn't
> 
> #define ONES (SIZE_MAX/255)
> 
> be clearer

it is two chars shorter at least :)

the unsigned -1 idiom is used a lot in musl anyway

the really clear definition would be (size_t)0x0101..01 but it
depends on the word size

btw i was going to suggest to just use unsigned long everywhere
so no size_t, uintptr_t and stdint.h since it is guaranteed to
do the right thing due to the syscall abi..

then realized that there is x32 and i'm wondering what's the
right thing there, since pointer, long, size_t, wordsize are all
32bit, but it can use 64bit registers..
(ie either using unsigned long or size_t etc would mean 32bit
word at a time in the string functions, which is fine but probably
not the best)

Re: [PATCH] String: expand to word size && refactor || refactor

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]

Am Sonntag, den 07.04.2013, 15:29 +0200 schrieb Szabolcs Nagy:
> * Jens Gustedt <jens.gustedt@inria.fr> [2013-04-07 11:38:48 +0200]:
> > Am Sonntag, den 07.04.2013, 11:23 +0200 schrieb Szabolcs Nagy:
> > > #define ONES ((size_t)-1/255)
> > 
> > just a nitpick, wouldn't
> > 
> > #define ONES (SIZE_MAX/255)
> > 
> > be clearer
> 
> it is two chars shorter at least :)
> 
> the unsigned -1 idiom is used a lot in musl anyway

hm, for macros I really would be more cautious. Couldn't one day such
a macro end up in a #if directive? Then using the named macro would be
better since this then gives the same value for the
preprocessor.

(size_t)-1 is wrong because this would be -1 for the preprocessor. So
it should at least be (size_t)-1U to ensure that this is an unsigned
integer even in the preprocessor. But this would give a different
value, still, if size_t is narrower than uintmax_t.

> the really clear definition would be (size_t)0x0101..01 but it
> depends on the word size

that should be (size_t)+0x0101..01 such that it can be evaluated in
the preprocessor.

Jens

-- 
:: INRIA Nancy Grand Est :: http://www.loria.fr/~gustedt/   ::
:: AlGorille ::::::::::::::: office Nancy : +33 383593090   ::
:: ICube :::::::::::::: office Strasbourg : +33 368854536   ::
:: ::::::::::::::::::::::::::: gsm France : +33 651400183   ::
:: :::::::::::::::::::: gsm international : +49 15737185122 ::



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] String: expand to word size && refactor || refactor

[Szabolcs Nagy]

* Jens Gustedt <jens.gustedt@inria.fr> [2013-04-07 16:31:37 +0200]:
> > 
> > the unsigned -1 idiom is used a lot in musl anyway
> 
> hm, for macros I really would be more cautious. Couldn't one day such
> a macro end up in a #if directive? Then using the named macro would be
> better since this then gives the same value for the
> preprocessor.

yes this is clearly not preprocessor safe but that was not the goal

it's annoying that the preprocessor handles numbers as [u]intmax_t,
otherwise no bits/stdint.h or bits/limits.h were needed, you could
define all the limits in terms of -1U, -1UL, -1ULL

one could say that cpp is simpler this way but it still has to know
a lot of things eg wide characters should work: musl uses
#if L'0'-1 > 0 to determine if wchar_t is unsigned

> > the really clear definition would be (size_t)0x0101..01 but it
> > depends on the word size
> 
> that should be (size_t)+0x0101..01 such that it can be evaluated in
> the preprocessor.

nice trick..