[musl] [C23 const 0/2] some interfaces become type-generic

[musl] [C23 const 0/2] some interfaces become type-generic

[Jens Gustedt]

In C23, some interfaces become type-generic with the aim that they
respect the const-contract for their pointer parameters.

With the functions alone, non-qualified pointers of buffers that are
`const`-qualified can escape. The type-generic functions/macros at an
additional security here, that compilers may then check.

This code needs `_Generic`, so it only works for compilers with at
least C11. Other compilers should still see the function interfaces as
before.

Jens Gustedt (2):
  C23: change bsearch to a macro that respects the const contract
  C23: change string.h and wchar.h interfaces to macros that respects
    the const contract

 include/stdlib.h     | 12 +++++++++-
 include/string.h     | 54 ++++++++++++++++++++++++++++++++++++++-----
 include/wchar.h      | 55 +++++++++++++++++++++++++++++++++++++++-----
 src/include/stdlib.h |  2 ++
 src/include/string.h |  6 +++++
 src/include/wchar.h  |  6 +++++
 src/stdlib/bsearch.c |  2 +-
 src/string/memchr.c  |  2 +-
 src/string/strchr.c  |  2 +-
 src/string/strpbrk.c |  2 +-
 src/string/strrchr.c |  2 +-
 src/string/strstr.c  |  2 +-
 src/string/wcschr.c  |  2 +-
 src/string/wcspbrk.c |  2 +-
 src/string/wcsrchr.c |  2 +-
 src/string/wcsstr.c  |  2 +-
 src/string/wmemchr.c |  2 +-
 17 files changed, 133 insertions(+), 24 deletions(-)

-- 
2.34.1

[musl] [C23 const 1/2] C23: change bsearch to a macro that respects the const contract

[Jens Gustedt]

This adds a macro interface to stdlib that has an additional cast of
the return value to `void const*` for the case that the argument to
the call was also const-qualified. Nothing changes for the function
itself, only the identifier has to be protected with (), such that the
macro does not expand for the function declaration or definition.
---
 include/stdlib.h     | 12 +++++++++++-
 src/include/stdlib.h |  2 ++
 src/stdlib/bsearch.c |  2 +-
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/include/stdlib.h b/include/stdlib.h
index 68ccd467..f5281777 100644
--- a/include/stdlib.h
+++ b/include/stdlib.h
@@ -60,7 +60,17 @@ char *getenv (const char *);
 
 int system (const char *);
 
-void *bsearch (const void *, const void *, size_t, size_t, int (*)(const void *, const void *));
+void * (bsearch) (const void *, const void *, size_t, size_t, int (*)(const void *, const void *));
+#if __STDC_VERSION__ > 201112L
+# define bsearch(K, B, N, S, C)                                         \
+	_Generic(                                                       \
+		/* ensure conversion to a void pointer */               \
+		1 ? (B) : (void*)1,                                     \
+		void const*: (void const*)bsearch((K), (void const*)(B), (N), (S), (C)), \
+		/* volatile qualification of *B is an error for this call */ \
+		default:     bsearch((K), (B), (N), (S), (C))           \
+)
+#endif
 void qsort (void *, size_t, size_t, int (*)(const void *, const void *));
 
 int abs (int);
diff --git a/src/include/stdlib.h b/src/include/stdlib.h
index 812b04de..f0b03df9 100644
--- a/src/include/stdlib.h
+++ b/src/include/stdlib.h
@@ -16,4 +16,6 @@ hidden void *__libc_calloc(size_t, size_t);
 hidden void *__libc_realloc(void *, size_t);
 hidden void __libc_free(void *);
 
+#undef bsearch
+
 #endif
diff --git a/src/stdlib/bsearch.c b/src/stdlib/bsearch.c
index fe050ea3..4f62ea37 100644
--- a/src/stdlib/bsearch.c
+++ b/src/stdlib/bsearch.c
@@ -1,6 +1,6 @@
 #include <stdlib.h>
 
-void *bsearch(const void *key, const void *base, size_t nel, size_t width, int (*cmp)(const void *, const void *))
+void *(bsearch)(const void *key, const void *base, size_t nel, size_t width, int (*cmp)(const void *, const void *))
 {
 	void *try;
 	int sign;
-- 
2.34.1

[musl] [C23 const 2/2] C23: change string.h and wchar.h interfaces to macros that respects the const contract

[Jens Gustedt]

This adds a macro interfaces to those string functions that search for
a string position. This has an additional cast of the return value to
`void const*` for the case that the argument to the call was also
const-qualified. Nothing changes for the correspondin function itself,
only the identifier has to be protected with (), such that the macro
does not expand for the function declaration or definition.
---
 include/string.h     | 54 ++++++++++++++++++++++++++++++++++++++-----
 include/wchar.h      | 55 +++++++++++++++++++++++++++++++++++++++-----
 src/include/string.h |  6 +++++
 src/include/wchar.h  |  6 +++++
 src/string/memchr.c  |  2 +-
 src/string/strchr.c  |  2 +-
 src/string/strpbrk.c |  2 +-
 src/string/strrchr.c |  2 +-
 src/string/strstr.c  |  2 +-
 src/string/wcschr.c  |  2 +-
 src/string/wcspbrk.c |  2 +-
 src/string/wcsrchr.c |  2 +-
 src/string/wcsstr.c  |  2 +-
 src/string/wmemchr.c |  2 +-
 14 files changed, 119 insertions(+), 22 deletions(-)

diff --git a/include/string.h b/include/string.h
index 7df7a402..05019c03 100644
--- a/include/string.h
+++ b/include/string.h
@@ -28,7 +28,54 @@ void *memcpy (void *__restrict, const void *__restrict, size_t);
 void *memmove (void *, const void *, size_t);
 void *memset (void *, int, size_t);
 int memcmp (const void *, const void *, size_t);
-void *memchr (const void *, int, size_t);
+
+void *(memchr) (const void *, int, size_t);
+char *(strchr) (const char *, int);
+char *(strrchr) (const char *, int);
+char *(strpbrk) (const char *, const char *);
+char *(strstr) (const char *, const char *);
+#if __STDC_VERSION__ > 201112L
+# define memchr(S, C, N)                                                \
+	_Generic(                                                       \
+		/* ensure conversion to a void pointer */               \
+		1 ? (S) : (void*)1,                                     \
+		void const*: (void const*)memchr((void const*)(S), (C), (N)), \
+		/* volatile qualification of *S is an error for this call */ \
+		default:     memchr((S), (C), (N))                      \
+)
+# define strchr(S, C)                                                   \
+	_Generic(                                                       \
+		/* ensure conversion to a void pointer */               \
+		1 ? (S) : (void*)1,                                     \
+		void const*: (char const*)strchr((char const*){ (S) }, (C)), \
+		/* volatile qualification of *S is an error for this call */ \
+		default:     strchr((S), (C))                           \
+)
+# define strrchr(S, C)                                                   \
+	_Generic(                                                       \
+		/* ensure conversion to a void pointer */               \
+		1 ? (S) : (void*)1,                                     \
+		void const*: (char const*)strrchr((char const*){ (S) }, (C)), \
+		/* volatile qualification of *S is an error for this call */ \
+		default:     strrchr((S), (C))                           \
+)
+# define strpbrk(S, A)                                                  \
+	_Generic(                                                       \
+		/* ensure conversion to a void pointer */               \
+		1 ? (S) : (void*)1,                                     \
+		void const*: (char const*)strpbrk((char const*){ (S) }, (A)), \
+		/* volatile qualification of *S is an error for this call */ \
+		default:     strpbrk((S), (A))                          \
+)
+# define strstr(H, N)                                                   \
+	_Generic(                                                       \
+		/* ensure conversion to a void pointer */               \
+		1 ? (H) : (void*)1,                                     \
+		void const*: (char const*)strstr((char const*){ (H) }, (N)), \
+		/* volatile qualification of *S is an error for this call */ \
+		default:     strstr((H), (N))                           \
+)
+#endif
 
 char *strcpy (char *__restrict, const char *__restrict);
 char *strncpy (char *__restrict, const char *__restrict, size_t);
@@ -42,13 +89,8 @@ int strncmp (const char *, const char *, size_t);
 int strcoll (const char *, const char *);
 size_t strxfrm (char *__restrict, const char *__restrict, size_t);
 
-char *strchr (const char *, int);
-char *strrchr (const char *, int);
-
 size_t strcspn (const char *, const char *);
 size_t strspn (const char *, const char *);
-char *strpbrk (const char *, const char *);
-char *strstr (const char *, const char *);
 char *strtok (char *__restrict, const char *__restrict);
 
 size_t strlen (const char *);
diff --git a/include/wchar.h b/include/wchar.h
index ed5d774d..3816a7cd 100644
--- a/include/wchar.h
+++ b/include/wchar.h
@@ -61,21 +61,64 @@ int wcsncmp (const wchar_t *, const wchar_t *, size_t);
 int wcscoll(const wchar_t *, const wchar_t *);
 size_t wcsxfrm (wchar_t *__restrict, const wchar_t *__restrict, size_t);
 
-wchar_t *wcschr (const wchar_t *, wchar_t);
-wchar_t *wcsrchr (const wchar_t *, wchar_t);
-
 size_t wcscspn (const wchar_t *, const wchar_t *);
 size_t wcsspn (const wchar_t *, const wchar_t *);
-wchar_t *wcspbrk (const wchar_t *, const wchar_t *);
 
 wchar_t *wcstok (wchar_t *__restrict, const wchar_t *__restrict, wchar_t **__restrict);
 
 size_t wcslen (const wchar_t *);
 
-wchar_t *wcsstr (const wchar_t *__restrict, const wchar_t *__restrict);
 wchar_t *wcswcs (const wchar_t *, const wchar_t *);
 
-wchar_t *wmemchr (const wchar_t *, wchar_t, size_t);
+wchar_t *(wmemchr) (const wchar_t *, wchar_t, size_t);
+wchar_t *(wcschr) (const wchar_t *, wchar_t);
+wchar_t *(wcsrchr) (const wchar_t *, wchar_t);
+wchar_t *(wcspbrk) (const wchar_t *, const wchar_t *);
+wchar_t *(wcsstr) (const wchar_t *__restrict, const wchar_t *__restrict);
+#if __STDC_VERSION__ > 201112L
+# define wmemchr(S, C, N)                                               \
+	_Generic(                                                       \
+		/* ensure conversion to a void pointer */               \
+		1 ? (S) : (void*)1,                                     \
+		void const*: (wchar_t const*)wmemchr((wchar_t const*){ (S) }, (C), (N)), \
+		/* volatile qualification of *S is an error for this call */ \
+		default:     wmemchr((S), (C), (N))                     \
+)
+# define wcschr(S, C)                                                   \
+	_Generic(                                                       \
+		/* ensure conversion to a void pointer */               \
+		1 ? (S) : (void*)1,                                     \
+		void const*: (wchar_t const*)wcschr((wchar_t const*){ (S) }, (C)), \
+		/* volatile qualification of *S is an error for this call */ \
+		default:     wcschr((S), (C))                           \
+)
+# define wcsrchr(S, C)                                                  \
+	_Generic(                                                       \
+		/* ensure conversion to a void pointer */               \
+		1 ? (S) : (void*)1,                                     \
+		void const*: (wchar_t const*)wcsrchr((wchar_t const*){ (S) }, (C)), \
+		/* volatile qualification of *S is an error for this call */ \
+		default:     wcsrchr((S), (C))                          \
+)
+# define wcspbrk(S, A)                                                  \
+	_Generic(                                                       \
+		/* ensure conversion to a void pointer */               \
+		1 ? (S) : (void*)1,                                     \
+		void const*: (wchar_t const*)wcspbrk((wchar_t const*){ (S) }, (A)), \
+		/* volatile qualification of *S is an error for this call */ \
+		default:     wcspbrk((S), (A))                          \
+)
+# define wcsstr(H, N)                                                   \
+	_Generic(                                                       \
+		/* ensure conversion to a void pointer */               \
+		1 ? (H) : (void*)1,                                     \
+		void const*: (wchar_t const*)wcsstr((wchar_t const*){ (H) }, (N)), \
+		/* volatile qualification of *S is an error for this call */ \
+		default:     wcsstr((H), (N))                           \
+)
+#endif
+
+
 int wmemcmp (const wchar_t *, const wchar_t *, size_t);
 wchar_t *wmemcpy (wchar_t *__restrict, const wchar_t *__restrict, size_t);
 wchar_t *wmemmove (wchar_t *, const wchar_t *, size_t);
diff --git a/src/include/string.h b/src/include/string.h
index 2133b5c1..f536c26b 100644
--- a/src/include/string.h
+++ b/src/include/string.h
@@ -8,4 +8,10 @@ hidden char *__stpcpy(char *, const char *);
 hidden char *__stpncpy(char *, const char *, size_t);
 hidden char *__strchrnul(const char *, int);
 
+#undef memchr
+#undef strchr
+#undef strrchr
+#undef strpbrk
+#undef strstr
+
 #endif
diff --git a/src/include/wchar.h b/src/include/wchar.h
index 79f5d0e7..dcd1cce1 100644
--- a/src/include/wchar.h
+++ b/src/include/wchar.h
@@ -5,5 +5,11 @@
 
 #include "../../include/wchar.h"
 
+#undef wmemchr
+#undef wcschr
+#undef wcsrchr
+#undef wcspbrk
+#undef wcsstr
+
 #endif
 
diff --git a/src/string/memchr.c b/src/string/memchr.c
index 65f0d789..f11c0573 100644
--- a/src/string/memchr.c
+++ b/src/string/memchr.c
@@ -8,7 +8,7 @@
 #define HIGHS (ONES * (UCHAR_MAX/2+1))
 #define HASZERO(x) ((x)-ONES & ~(x) & HIGHS)
 
-void *memchr(const void *src, int c, size_t n)
+void *(memchr)(const void *src, int c, size_t n)
 {
 	const unsigned char *s = src;
 	c = (unsigned char)c;
diff --git a/src/string/strchr.c b/src/string/strchr.c
index 3cbc828b..3a86beeb 100644
--- a/src/string/strchr.c
+++ b/src/string/strchr.c
@@ -1,6 +1,6 @@
 #include <string.h>
 
-char *strchr(const char *s, int c)
+char *(strchr)(const char *s, int c)
 {
 	char *r = __strchrnul(s, c);
 	return *(unsigned char *)r == (unsigned char)c ? r : 0;
diff --git a/src/string/strpbrk.c b/src/string/strpbrk.c
index 55947c64..0941fc51 100644
--- a/src/string/strpbrk.c
+++ b/src/string/strpbrk.c
@@ -1,6 +1,6 @@
 #include <string.h>
 
-char *strpbrk(const char *s, const char *b)
+char *(strpbrk)(const char *s, const char *b)
 {
 	s += strcspn(s, b);
 	return *s ? (char *)s : 0;
diff --git a/src/string/strrchr.c b/src/string/strrchr.c
index 98ad1b04..908fd8d4 100644
--- a/src/string/strrchr.c
+++ b/src/string/strrchr.c
@@ -1,6 +1,6 @@
 #include <string.h>
 
-char *strrchr(const char *s, int c)
+char *(strrchr)(const char *s, int c)
 {
 	return __memrchr(s, c, strlen(s) + 1);
 }
diff --git a/src/string/strstr.c b/src/string/strstr.c
index 96657bc2..5c622a82 100644
--- a/src/string/strstr.c
+++ b/src/string/strstr.c
@@ -135,7 +135,7 @@ static char *twoway_strstr(const unsigned char *h, const unsigned char *n)
 	}
 }
 
-char *strstr(const char *h, const char *n)
+char *(strstr)(const char *h, const char *n)
 {
 	/* Return immediately on empty needle */
 	if (!n[0]) return (char *)h;
diff --git a/src/string/wcschr.c b/src/string/wcschr.c
index 8dfc2f31..2bf7a111 100644
--- a/src/string/wcschr.c
+++ b/src/string/wcschr.c
@@ -1,6 +1,6 @@
 #include <wchar.h>
 
-wchar_t *wcschr(const wchar_t *s, wchar_t c)
+wchar_t *(wcschr)(const wchar_t *s, wchar_t c)
 {
 	if (!c) return (wchar_t *)s + wcslen(s);
 	for (; *s && *s != c; s++);
diff --git a/src/string/wcspbrk.c b/src/string/wcspbrk.c
index 0c72c197..eb76b5ff 100644
--- a/src/string/wcspbrk.c
+++ b/src/string/wcspbrk.c
@@ -1,6 +1,6 @@
 #include <wchar.h>
 
-wchar_t *wcspbrk(const wchar_t *s, const wchar_t *b)
+wchar_t *(wcspbrk)(const wchar_t *s, const wchar_t *b)
 {
 	s += wcscspn(s, b);
 	return *s ? (wchar_t *)s : NULL;
diff --git a/src/string/wcsrchr.c b/src/string/wcsrchr.c
index 8961b9e2..889303f3 100644
--- a/src/string/wcsrchr.c
+++ b/src/string/wcsrchr.c
@@ -1,6 +1,6 @@
 #include <wchar.h>
 
-wchar_t *wcsrchr(const wchar_t *s, wchar_t c)
+wchar_t *(wcsrchr)(const wchar_t *s, wchar_t c)
 {
 	const wchar_t *p;
 	for (p=s+wcslen(s); p>=s && *p!=c; p--);
diff --git a/src/string/wcsstr.c b/src/string/wcsstr.c
index 4caaef3c..8f27dbea 100644
--- a/src/string/wcsstr.c
+++ b/src/string/wcsstr.c
@@ -90,7 +90,7 @@ static wchar_t *twoway_wcsstr(const wchar_t *h, const wchar_t *n)
 	}
 }
 
-wchar_t *wcsstr(const wchar_t *restrict h, const wchar_t *restrict n)
+wchar_t *(wcsstr)(const wchar_t *restrict h, const wchar_t *restrict n)
 {
 	/* Return immediately on empty needle or haystack */
 	if (!n[0]) return (wchar_t *)h;
diff --git a/src/string/wmemchr.c b/src/string/wmemchr.c
index 2bc2c270..3d761488 100644
--- a/src/string/wmemchr.c
+++ b/src/string/wmemchr.c
@@ -1,6 +1,6 @@
 #include <wchar.h>
 
-wchar_t *wmemchr(const wchar_t *s, wchar_t c, size_t n)
+wchar_t *(wmemchr)(const wchar_t *s, wchar_t c, size_t n)
 {
 	for (; n && *s != c; n--, s++);
 	return n ? (wchar_t *)s : 0;
-- 
2.34.1

Re: [musl] [C23 const 1/2] C23: change bsearch to a macro that respects the const contract

[Nat!]

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

I can't help but comment on these changes. :)

I think it's sort of obvious, that these macros increase code 
brittleness due to now multiple execution of macro arguments vs. a 
single execution in a function call. Weighing this versus the loss of 
const qualification (on my personal importance scale from 0-10a clear 
0), it is not a worthwhile tradeoff.

Ciao
    Nat!


Am 25.05.23 um 16:44 schrieb Jens Gustedt:
> This adds a macro interface to stdlib that has an additional cast of
> the return value to `void const*` for the case that the argument to
> the call was also const-qualified. Nothing changes for the function
> itself, only the identifier has to be protected with (), such that the
> macro does not expand for the function declaration or definition.
> ---
>   include/stdlib.h     | 12 +++++++++++-
>   src/include/stdlib.h |  2 ++
>   src/stdlib/bsearch.c |  2 +-
>   3 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/include/stdlib.h b/include/stdlib.h
> index 68ccd467..f5281777 100644
> --- a/include/stdlib.h
> +++ b/include/stdlib.h
> @@ -60,7 +60,17 @@ char *getenv (const char *);
>   
>   int system (const char *);
>   
> -void *bsearch (const void *, const void *, size_t, size_t, int (*)(const void *, const void *));
> +void * (bsearch) (const void *, const void *, size_t, size_t, int (*)(const void *, const void *));
> +#if __STDC_VERSION__ > 201112L
> +# define bsearch(K, B, N, S, C)                                         \
> +	_Generic(                                                       \
> +		/* ensure conversion to a void pointer */               \
> +		1 ? (B) : (void*)1,                                     \
> +		void const*: (void const*)bsearch((K), (void const*)(B), (N), (S), (C)), \
> +		/* volatile qualification of *B is an error for this call */ \
> +		default:     bsearch((K), (B), (N), (S), (C))           \
> +)
> +#endif
>   void qsort (void *, size_t, size_t, int (*)(const void *, const void *));
>   
>   int abs (int);
> diff --git a/src/include/stdlib.h b/src/include/stdlib.h
> index 812b04de..f0b03df9 100644
> --- a/src/include/stdlib.h
> +++ b/src/include/stdlib.h
> @@ -16,4 +16,6 @@ hidden void *__libc_calloc(size_t, size_t);
>   hidden void *__libc_realloc(void *, size_t);
>   hidden void __libc_free(void *);
>   
> +#undef bsearch
> +
>   #endif
> diff --git a/src/stdlib/bsearch.c b/src/stdlib/bsearch.c
> index fe050ea3..4f62ea37 100644
> --- a/src/stdlib/bsearch.c
> +++ b/src/stdlib/bsearch.c
> @@ -1,6 +1,6 @@
>   #include <stdlib.h>
>   
> -void *bsearch(const void *key, const void *base, size_t nel, size_t width, int (*cmp)(const void *, const void *))
> +void *(bsearch)(const void *key, const void *base, size_t nel, size_t width, int (*cmp)(const void *, const void *))
>   {
>   	void *try;
>   	int sign;

[-- Attachment #2: Type: text/html, Size: 3511 bytes --]

Re: [musl] [C23 const 1/2] C23: change bsearch to a macro that respects the const contract

[NRK]

On Fri, May 26, 2023 at 01:29:31PM +0200, Nat! wrote:
> I think it's sort of obvious, that these macros increase code brittleness
> due to now multiple execution of macro arguments vs. a single execution in a
> function call.

It would be heavily surprising if the controlling expression of _Generic
was evaluated. Similar to `sizeof`, it only needs to know the type of
the expression and thus doesn't require evaluation (only exception being
VLAs in a sizeof).

And looking at cppreference, it seems that the controlling expression
indeed isn't evaluated: https://en.cppreference.com/w/c/language/generic#Notes

| The controlling-expression and the expressions of the selections that
| are not chosen are never evaluated.

However, there is one thing that I don't quite understand about this
patch:

> +		void const*: (void const*)bsearch((K), (void const*)(B), (N), (S), (C)), \

What's with the `(void const*)(B)` cast? It's already determined to be
`void const *` via _Generic.

- NRK

Re: [musl] [C23 const 1/2] C23: change bsearch to a macro that respects the const contract

[Jₑₙₛ Gustedt]

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


on Fri, 26 May 2023 23:20:37 +0600 you (NRK <nrk@disroot.org>) wrote:

> On Fri, May 26, 2023 at 01:29:31PM +0200, Nat! wrote:
> > I think it's sort of obvious, that these macros increase code
> > brittleness due to now multiple execution of macro arguments vs. a
> > single execution in a function call.  
> 
> It would be heavily surprising if the controlling expression of
> _Generic was evaluated. Similar to `sizeof`, it only needs to know
> the type of the expression and thus doesn't require evaluation (only
> exception being VLAs in a sizeof).

Yes, exactly, that's the idea. And VLA (and similar) are not allowed
for the controling expression of a generic selection, so this problem
does not arise, there.

> | The controlling-expression and the expressions of the selections
> that | are not chosen are never evaluated.
> 
> However, there is one thing that I don't quite understand about this
> patch:
> 
> > +		void const*: (void const*)bsearch((K), (void
> > const*)(B), (N), (S), (C)), \  
> 
> What's with the `(void const*)(B)` cast? It's already determined to be
> `void const *` via _Generic.

Even if not evaluated, all branches have to be syntactically and
semantically correct. If you don't use these kind of casts, you may get
a lot of warnings.

This "feature" makes the use of these beast a bit subtle, sometimes.

(Originally, generic selection had been designed to chose between
different function pointers similar as in <tgmath.h> to implement
something like overloading.)


Thanks
Jₑₙₛ

-- 
:: ICube :::::::::::::::::::::::::::::: deputy director ::
:: Université de Strasbourg :::::::::::::::::::::: ICPS ::
:: INRIA Nancy Grand Est :::::::::::::::::::::::: Camus ::
:: :::::::::::::::::::::::::::::::::::: ☎ +33 368854536 ::
:: https://icube-icps.unistra.fr/index.php/Jens_Gustedt ::

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [musl] [C23 const 1/2] C23: change bsearch to a macro that respects the const contract

[Rich Felker]

On Fri, May 26, 2023 at 09:29:51PM +0200, Jₑₙₛ Gustedt wrote:
> 
> on Fri, 26 May 2023 23:20:37 +0600 you (NRK <nrk@disroot.org>) wrote:
> 
> > On Fri, May 26, 2023 at 01:29:31PM +0200, Nat! wrote:
> > > I think it's sort of obvious, that these macros increase code
> > > brittleness due to now multiple execution of macro arguments vs. a
> > > single execution in a function call.  
> > 
> > It would be heavily surprising if the controlling expression of
> > _Generic was evaluated. Similar to `sizeof`, it only needs to know
> > the type of the expression and thus doesn't require evaluation (only
> > exception being VLAs in a sizeof).
> 
> Yes, exactly, that's the idea. And VLA (and similar) are not allowed
> for the controling expression of a generic selection, so this problem
> does not arise, there.

Yes. And anyway, having them is a requirement of the standard, not
some optional enhancement we're making.

> > | The controlling-expression and the expressions of the selections
> > that | are not chosen are never evaluated.
> > 
> > However, there is one thing that I don't quite understand about this
> > patch:
> > 
> > > +		void const*: (void const*)bsearch((K), (void
> > > const*)(B), (N), (S), (C)), \  
> > 
> > What's with the `(void const*)(B)` cast? It's already determined to be
> > `void const *` via _Generic.
> 
> Even if not evaluated, all branches have to be syntactically and
> semantically correct. If you don't use these kind of casts, you may get
> a lot of warnings.

Could you elaborate on how such a warning could arise? bsearch (the
function) takes const void *, and any object pointer type (assuming no
other qualification like volatile on the pointed-to type) converts
implicitly to const void *. So I don't think it's possible.

> This "feature" makes the use of these beast a bit subtle, sometimes.
> 
> (Originally, generic selection had been designed to chose between
> different function pointers similar as in <tgmath.h> to implement
> something like overloading.)

In that case it may have been needed, if the alt function took void *
rather than const void *. However, thankfully that path wasn't taken,
so I think this is just a remnant of an untaken path in the
standardization process and should be dropped.

I'll go ahead and follow up here rather than branching this thread
elsewhere too: public headers are supposed to avoid creative content
like comments. Once you remove the unneeded casts, comments, etc.
these _Generics become much more compact and don't need the many-line,
aligned-\ macro thing we don't generally do in musl.

Rich

Re: [musl] [C23 const 1/2] C23: change bsearch to a macro that respects the const contract

[Jₑₙₛ Gustedt]

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

Rich,

on Fri, 26 May 2023 16:09:42 -0400 you (Rich Felker <dalias@libc.org>)
wrote:

> I'll go ahead and follow up here rather than branching this thread
> elsewhere too: public headers are supposed to avoid creative content
> like comments. Once you remove the unneeded casts, comments, etc.
> these _Generics become much more compact and don't need the many-line,
> aligned-\ macro thing we don't generally do in musl.

Well, such a policy might be outdated. Nowadays compilers are quite
good in tracing errors back into macro defintions. Especially for
generic selection it is then really helpful for the user that they can
trace this down to a particular line of the macro that ideally
corresponds to exactly their selection that went wrong.

(Admittedly that's not a big deal here, yet, because this has only two
cases. But in general, when we have more of that stuff, having good
feedback from the compiler into the macro definition is very
valuable.)

> Could you elaborate on how such a warning could arise? bsearch (the
> function) takes const void *, and any object pointer type (assuming no
> other qualification like volatile on the pointed-to type) converts
> implicitly to const void *. So I don't think it's possible.

The `volatile` is indeed the problem here. If an application would do
that, without the cast they would see the error on the wrong
selection, which might be quite confusing.

Thanks
Jₑₙₛ

-- 
:: ICube :::::::::::::::::::::::::::::: deputy director ::
:: Université de Strasbourg :::::::::::::::::::::: ICPS ::
:: INRIA Nancy Grand Est :::::::::::::::::::::::: Camus ::
:: :::::::::::::::::::::::::::::::::::: ☎ +33 368854536 ::
:: https://icube-icps.unistra.fr/index.php/Jens_Gustedt ::

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]