NetBSD Problem Report #56241

From www@netbsd.org  Wed Jun  9 00:23:25 2021
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id DADFD1A921F
	for <gnats-bugs@gnats.NetBSD.org>; Wed,  9 Jun 2021 00:23:25 +0000 (UTC)
Message-Id: <20210609002324.7538E1A9239@mollari.NetBSD.org>
Date: Wed,  9 Jun 2021 00:23:24 +0000 (UTC)
From: doremylover123@gmail.com
Reply-To: doremylover123@gmail.com
To: gnats-bugs@NetBSD.org
Subject: Many string functions do not confirm to the C99 standard
X-Send-Pr-Version: www-1.0

>Number:         56241
>Category:       lib
>Synopsis:       Many string functions do not confirm to the C99 standard
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Wed Jun 09 00:25:00 +0000 2021
>Last-Modified:  Wed Jun 09 14:25:01 +0000 2021
>Originator:     Seija K.
>Release:        9.2
>Organization:
>Environment:
>Description:
The restrict qualifiers are missing in many of these functions. I added a patch that adds them and updates the documentation with this.
>How-To-Repeat:

>Fix:
A diff:

diff --git a/games/arithmetic/arithmetic.c b/games/arithmetic/arithmetic.c
index 743948e68703..1ee2cd04f83a 100644
--- a/games/arithmetic/arithmetic.c
+++ b/games/arithmetic/arithmetic.c
@@ -145,7 +145,7 @@ main(int argc, char **argv)

 	/* Now ask the questions. */
 	for (;;) {
-		for (cnt = NQUESTS; cnt--;)
+		for (cnt = NQUESTS; cnt; cnt--)
 			if (problem() == EOF)
 				exit(0);
 		showstats(0);
diff --git a/include/string.h b/include/string.h
index f61004e30b2f..12efcf022cd6 100644
--- a/include/string.h
+++ b/include/string.h
@@ -69,14 +69,14 @@ char	*strstr(const char *, const char *);
 char	*strtok(char * __restrict, const char * __restrict);
 #if (_POSIX_C_SOURCE - 0 >= 199506L) || (_XOPEN_SOURCE - 0 >= 500) || \
     defined(_REENTRANT) || defined(_NETBSD_SOURCE)
-char	*strtok_r(char *, const char *, char **);
+char	*strtok_r(char * __restrict, const char * __restrict, char ** __restrict);
 int	 strerror_r(int, char *, size_t);
 #endif /* _POSIX_C_SOURCE >= 199506 || XOPEN_SOURCE >= 500 || ... */
 size_t	 strxfrm(char * __restrict, const char * __restrict, size_t);

 #if (_POSIX_C_SOURCE - 0 >= 200112L) || defined(_XOPEN_SOURCE) || \
     defined(_NETBSD_SOURCE)
-void	*memccpy(void *, const void *, int, size_t);
+void	*memccpy(void * __restrict, const void * __restrict, int, size_t);
 char	*strdup(const char *);
 #endif

@@ -100,8 +100,8 @@ __BEGIN_DECLS
 void	*memmem(const void *, size_t, const void *, size_t);
 char	*strcasestr(const char *, const char *);
 char	*strchrnul(const char *, int);
-size_t	 strlcat(char *, const char *, size_t);
-size_t	 strlcpy(char *, const char *, size_t);
+size_t	 strlcat(char * __restrict, const char * __restrict, size_t);
+size_t	 strlcpy(char * __restrict, const char * __restrict, size_t);
 char	*strsep(char **, const char *);
 char	*stresep(char **, const char *, int);
 char	*strnstr(const char *, const char *, size_t);
diff --git a/lib/libc/stdlib/malloc.c b/lib/libc/stdlib/malloc.c
index 12402ae766a5..f9cad1b177b5 100644
--- a/lib/libc/stdlib/malloc.c
+++ b/lib/libc/stdlib/malloc.c
@@ -379,7 +379,7 @@ map_pages(size_t pages)
 	/* Put back break point since we failed. */
 	if (brk(malloc_brk))
 	    wrterror("brk(2) failed [internal error]\n");
-	return 0;
+	return NULL;
     }

     return rresult;
@@ -824,12 +824,12 @@ irealloc(void *ptr, size_t size)

     if (idx < malloc_pageshift) {
 	wrtwarning("junk pointer, too low to make sense.\n");
-	return 0;
+	return NULL;
     }

     if (idx > last_idx) {
 	wrtwarning("junk pointer, too high to make sense.\n");
-	return 0;
+	return NULL;
     }

     mp = &page_dir[idx];
@@ -891,12 +891,13 @@ irealloc(void *ptr, size_t size)

     if (p != NULL) {
 	/* copy the lesser of the two sizes, and free the old one */
-	if (!size || !osize)
-	    ;
-	else if (osize < size)
-	    memcpy(p, ptr, osize);
-	else
-	    memcpy(p, ptr, size);
+	if (size && osize) { 
+	    if (osize < size)
+		memcpy(p, ptr, osize);
+	    else
+		memcpy(p, ptr, size);
+	}
+
 	ifree(ptr);
     } 
     return p;
diff --git a/lib/libc/string/memccpy.3 b/lib/libc/string/memccpy.3
index 9f6b54e08503..6460a948fee8 100644
--- a/lib/libc/string/memccpy.3
+++ b/lib/libc/string/memccpy.3
@@ -39,7 +39,7 @@
 .Sh SYNOPSIS
 .In string.h
 .Ft void *
-.Fn memccpy "void *dst" "const void *src" "int c" "size_t len"
+.Fn memccpy "void * restrict dst" "const void * restrict src" "int c" "size_t len"
 .Sh DESCRIPTION
 The
 .Fn memccpy
diff --git a/lib/libc/string/memccpy.c b/lib/libc/string/memccpy.c
index c08624161103..3ff6faeee03f 100644
--- a/lib/libc/string/memccpy.c
+++ b/lib/libc/string/memccpy.c
@@ -42,7 +42,7 @@ __RCSID("$NetBSD: memccpy.c,v 1.13 2012/06/25 22:32:46 abs Exp $");
 #include <string.h>

 void *
-memccpy(void *t, const void *f, int c, size_t n)
+memccpy(void * __restrict t, const void * __restrict f, int c, size_t n)
 {

 	_DIAGASSERT(t != 0);
@@ -51,7 +51,7 @@ memccpy(void *t, const void *f, int c, size_t n)
 	if (n) {
 		unsigned char *tp = t;
 		const unsigned char *fp = f;
-		unsigned char uc = c;
+		const unsigned char uc = c;
 		do {
 			if ((*tp++ = *fp++) == uc)
 				return (tp);
diff --git a/lib/libc/string/memrchr.c b/lib/libc/string/memrchr.c
index 92ef3289fbdf..9c7e5d064e82 100644
--- a/lib/libc/string/memrchr.c
+++ b/lib/libc/string/memrchr.c
@@ -43,7 +43,7 @@ memrchr(const void *s, int c, size_t n)

 	if (n != 0) {
 		const unsigned char *p = (const unsigned char *)s + n;
-		const unsigned char cmp = c;
+		const unsigned char cmp = (unsigned char)c;

 		do {
 			if (*--p == cmp)
diff --git a/lib/libc/string/stpcpy.c b/lib/libc/string/stpcpy.c
index 2b2d4075be3a..3e225f755cb4 100644
--- a/lib/libc/string/stpcpy.c
+++ b/lib/libc/string/stpcpy.c
@@ -50,7 +50,6 @@ __RCSID("$NetBSD: stpcpy.c,v 1.2 2013/11/06 21:05:27 tron Exp $");
 char *
 stpcpy(char * __restrict to, const char * __restrict from)
 {
-
 	for (; (*to = *from); ++from, ++to);
 	return(to);
 }
diff --git a/lib/libc/string/stpncpy.c b/lib/libc/string/stpncpy.c
index 9e3a4a147928..ecfb54e1f89e 100644
--- a/lib/libc/string/stpncpy.c
+++ b/lib/libc/string/stpncpy.c
@@ -41,11 +41,10 @@ __RCSID("$NetBSD: stpncpy.c,v 1.2 2013/11/06 21:05:27 tron Exp $");
 char *
 stpncpy(char * __restrict dst, const char * __restrict src, size_t n)
 {
-
-	for (; n--; dst++, src++) {
-		if (!(*dst = *src)) {
-			char *ret = dst;
-			while (n--)
+	for (; n; n--, dst++, src++) {
+		if ((*dst = *src) == '\0') {
+			char * const ret = dst;
+			for (; n; n--)
 				*++dst = '\0';
 			return (ret);
 		}
diff --git a/lib/libc/string/strndup.c b/lib/libc/string/strndup.c
index 28dd39c2b02c..96026f8661e9 100644
--- a/lib/libc/string/strndup.c
+++ b/lib/libc/string/strndup.c
@@ -65,7 +65,7 @@ strndup(const char *str, size_t n)
 	for (len = 0; len < n && str[len]; len++)
 		continue;

-	if (!(copy = malloc(len + 1)))
+	if ((copy = malloc(len + 1)) == NULL)
 		return (NULL);
 	memcpy(copy, str, len);
 	copy[len] = '\0';
diff --git a/lib/libc/string/strtok.3 b/lib/libc/string/strtok.3
index c6f5f6b58ad1..a16ae68e2e38 100644
--- a/lib/libc/string/strtok.3
+++ b/lib/libc/string/strtok.3
@@ -45,7 +45,7 @@
 .Ft char *
 .Fn strtok "char * restrict str" "const char * restrict sep"
 .Ft char *
-.Fn strtok_r "char *str" "const char *sep" "char **lasts"
+.Fn strtok_r "char * restrict str" "const char * restrict sep" "char ** restrict lasts"
 .Sh DESCRIPTION
 The
 .Fn strtok
@@ -142,7 +142,7 @@ The
 .Fn strtok
 function
 conforms to
-.St -ansiC .
+.St -isoC-99 .
 The
 .Fn strtok_r
 function conforms to
diff --git a/lib/libc/string/strtok.c b/lib/libc/string/strtok.c
index 4fc6214680c2..82773a7d10a0 100644
--- a/lib/libc/string/strtok.c
+++ b/lib/libc/string/strtok.c
@@ -42,7 +42,7 @@ __RCSID("$NetBSD: strtok.c,v 1.12 2004/10/27 19:12:31 dsl Exp $");
 #include <string.h>

 char *
-strtok(char *s, const char *delim)
+strtok(char * __restrict s, const char * __restrict delim)
 {
 	static char *lasts;

diff --git a/lib/libc/string/strtok_r.c b/lib/libc/string/strtok_r.c
index 9ed168b5ca0d..155e97ce9058 100644
--- a/lib/libc/string/strtok_r.c
+++ b/lib/libc/string/strtok_r.c
@@ -48,7 +48,7 @@ __weak_alias(strtok_r,_strtok_r)
 #endif

 char *
-strtok_r(char *s, const char *delim, char **lasts)
+strtok_r(char * __restrict s, const char * __restrict delim, char ** __restrict lasts)
 {
 	const char *spanp;
 	int c, sc;
diff --git a/lib/libc/string/wcslcat.c b/lib/libc/string/wcslcat.c
index a62a9c17ada2..0e1abd89722f 100644
--- a/lib/libc/string/wcslcat.c
+++ b/lib/libc/string/wcslcat.c
@@ -45,32 +45,32 @@ __RCSID("$NetBSD: wcslcat.c,v 1.3 2012/06/25 22:32:46 abs Exp $");
  * truncation occurred.
  */
 size_t
-wcslcat(wchar_t *dst, const wchar_t *src, size_t siz)
+wcslcat(wchar_t * __restrict dst, const wchar_t * __restrict src, size_t siz)
 {
-	wchar_t *d = dst;
-	const wchar_t *s = src;
-	size_t n = siz;
+	wchar_t * const d = dst;
+	const wchar_t * const s = src;
+	size_t n;
 	size_t dlen;

 	_DIAGASSERT(dst != NULL);
 	_DIAGASSERT(src != NULL);

 	/* Find the end of dst and adjust bytes left but don't go past end */
-	while (*d != '\0' && n-- != 0)
-		d++;
+	for (n = siz; *dst != '\0' && n != 0; n--)
+		dst++;
 	dlen = d - dst;
 	n = siz - dlen;

 	if (n == 0)
 		return(dlen + wcslen(s));
-	while (*s != '\0') {
+	while (*src != '\0') {
 		if (n != 1) {
-			*d++ = *s;
+			*dst++ = *src;
 			n--;
 		}
-		s++;
+		src++;
 	}
-	*d = '\0';
+	*dst = '\0';

-	return(dlen + (s - src));	/* count does not include NUL */
+	return(dlen + (src - s));	/* count does not include NUL */
 }
diff --git a/lib/libc/string/wmemcpy.c b/lib/libc/string/wmemcpy.c
index b648fb886aad..7e013cebf1d3 100644
--- a/lib/libc/string/wmemcpy.c
+++ b/lib/libc/string/wmemcpy.c
@@ -38,7 +38,7 @@ __RCSID("$NetBSD: wmemcpy.c,v 1.3 2012/06/25 22:32:46 abs Exp $");
 #include <wchar.h>

 wchar_t *
-wmemcpy(wchar_t *d, const wchar_t *s, size_t n)
+wmemcpy(wchar_t * __restrict d, const wchar_t * __restrict s, size_t n)
 {

 	_DIAGASSERT(d != NULL);
diff --git a/lib/libc/string/wmemset.c b/lib/libc/string/wmemset.c
index 597da33ad2fb..a891f5f5b5ad 100644
--- a/lib/libc/string/wmemset.c
+++ b/lib/libc/string/wmemset.c
@@ -39,15 +39,12 @@ __RCSID("$NetBSD: wmemset.c,v 1.3 2012/06/25 22:32:46 abs Exp $");
 wchar_t	*
 wmemset(wchar_t *s, wchar_t c, size_t n)
 {
-	size_t i;
-	wchar_t *p;
+	wchar_t * const p = s;

 	_DIAGASSERT(s != NULL);

-	p = (wchar_t *)s;
-	for (i = 0; i < n; i++) {
-		*p = c;
-		p++;
-	}
-	return s;
+	for (; n != 0; n--) 
+		*s++ = c;
+
+	return p;
 }

>Audit-Trail:
From: Joerg Sonnenberger <joerg@bec.de>
To: gnats-bugs@netbsd.org
Cc: lib-bug-people@netbsd.org, gnats-admin@netbsd.org,
	netbsd-bugs@netbsd.org
Subject: Re: lib/56241: Many string functions do not confirm to the C99
 standard
Date: Wed, 9 Jun 2021 16:24:11 +0200

 On Wed, Jun 09, 2021 at 12:25:00AM +0000, doremylover123@gmail.com wrote:
 > >Description:
 > The restrict qualifiers are missing in many of these functions. I added a patch that adds them and updates the documentation with this.

 The patch doesn't really match the description. Note that the
 implementation is entirely valid under C99 rules: restrict for function
 arguments is only restricting the valid arguments further. Conforming
 code does not change its behavior if the restrict is not present.

 Joerg

Home	PR Database Search