[v2] Remove strdup inlines
Commit Message
ping
From: Wilco Dijkstra
Sent: 12 December 2016 13:02
To: Zack Weinberg
Cc: libc-alpha@sourceware.org; nd
Subject: Re: [GLIBC][PATCH v2] Remove strdup inlines
Remove the str(n)dup inlines from string/bits/string2.h. Although inlining
calls with constant strings shows a small (~10%) performance gain, strdup is
typically used in error reporting code, so not performance critical.
Remove the now unused __need_malloc_and_calloc related defines from stdlib.h.
Avoid linknamespace and localplt failures by redirecting str(n)dup to GCC
builtins.
ChangeLog:
2015-12-12 Wilco Dijkstra <wdijkstr@arm.com>
* include/stdlib.h (__need_malloc_and_calloc): Remove uses.
(__Need_M_And_C) Remove define/undef.
* stdlib/stdlib.h (__need_malloc_and_calloc): Remove uses.
(__malloc_and_calloc_defined): Remove define.
* string/string.h (strdup): Use builtin.
(strndup): Likewise.
* string/bits/string2.h (__strdup): Remove define.
(strdup): Likewise.
(__strndup): Likewise.
(strndup): Likewise.
--
Comments
LGTM with some comments below:
On 09/02/2017 13:35, Wilco Dijkstra wrote:
> diff --git a/string/bits/string2.h b/string/bits/string2.h
> index 03af22cc27a33c81f36d56ddc52fce0a5ea81ece..b0be5f6a49024a0bedfc37e9ec2c0356e0e4fa09 100644
> --- a/string/bits/string2.h
> +++ b/string/bits/string2.h
> @@ -118,58 +118,6 @@
> #endif
>
>
> -/* We need the memory allocation functions for inline strdup().
> - Referring to stdlib.h (even minimally) is not allowed
> - in any of the tight standards compliant modes. */
> -#ifdef __USE_MISC
> -
> -# define __need_malloc_and_calloc
> -# include <stdlib.h>
> -
I think this patch is based on other ones, since this does not apply on
master.
> diff --git a/string/string.h b/string/string.h
> index b103e64912fe1904098e229ccb845bb2c5c10835..c251cc603a1dbb5bef469d4b71f90037612d36f0 100644
> --- a/string/string.h
> +++ b/string/string.h
> @@ -636,6 +636,15 @@ extern char *basename (const char *__filename) __THROW __nonnull ((1));
> # endif
> #endif
>
> +#if (defined __USE_XOPEN_EXTENDED || defined __USE_XOPEN2K8 \
> + || __GLIBC_USE (LIB_EXT2))
> +# define strdup(s) __builtin_strdup (s)
> +#endif
This is not suffice to avoid static link namespace issues, since gcc usually
issue a strdup for __builtin_strdup. The conform tests shows a lot of issues
for both strdup and strndup. I think a better option would just to redirect
it to __strdup and __strndup (since they are exported on libc.so).
Also for strdup I think __USE_XOPEN2K8 is not required, accordingly to manual
it is defined for
_SVID_SOURCE || _BSD_SOURCE || _XOPEN_SOURCE >= 500 || _XOPEN_SOURCE && _XOPEN_SOURCE_EXTENDED
And __USE_XOPEN_EXTENDED will be set for _XOPEN_SOURCE >= 500 or if
_XOPEN_SOURCE_EXTENDED is defined. At least all required defined are
suffice on conform testcases for just
#if (defined __USE_XOPEN_EXTENDED || __GLIBC_USE (LIB_EXT2))
> +
> +#if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2)
> +#define strndup(s,n) __builtin_strndup (s, n)
> +#endif
> +
> #if defined __USE_GNU && defined __OPTIMIZE__ \
> && defined __extern_always_inline && __GNUC_PREREQ (3,2)
> # if !defined _FORCE_INLINES && !defined _HAVE_STRING_ARCH_mempcpy
>
>
>
>
On Thu, 9 Feb 2017, Adhemerval Zanella wrote:
> Also for strdup I think __USE_XOPEN2K8 is not required, accordingly to manual
> it is defined for
>
> _SVID_SOURCE || _BSD_SOURCE || _XOPEN_SOURCE >= 500 || _XOPEN_SOURCE && _XOPEN_SOURCE_EXTENDED
strdup was XSI-shaded in the 2001 edition of POSIX but moved to CX-shaded
in the 2008 edition. That is, it should be declared for
_POSIX_C_SOURCE=200809L. That is, the existing __USE_XOPEN2K8 condition
(which means non-XSI 2008 edition of POSIX) is right.
> And __USE_XOPEN_EXTENDED will be set for _XOPEN_SOURCE >= 500 or if
> _XOPEN_SOURCE_EXTENDED is defined. At least all required defined are
> suffice on conform testcases for just
This indicates you've found a bug in the conform/ expectations - strdup
should be required there for POSIX2008.
@@ -1,16 +1,12 @@
 #ifndef _STDLIB_H
Â
-#ifdef __need_malloc_and_calloc
-#define __Need_M_And_C
-#endif
-
 #ifndef _ISOMAC
 # include <stddef.h>
 #endif
 #include <stdlib/stdlib.h>
Â
 /* Now define the internal interfaces. */
-#if !defined __Need_M_And_C && !defined _ISOMAC
+#if !defined _ISOMAC
 # include <sys/stat.h>
Â
 __BEGIN_DECLS
@@ -269,6 +265,4 @@ __END_DECLS
Â
 #endif
Â
-#undef __Need_M_And_C
-
 #endif /* include/stdlib.h */
@@ -26,15 +26,12 @@
Â
 /* Get size_t, wchar_t and NULL from <stddef.h>. */
 #define        __need_size_t
-#ifndef __need_malloc_and_calloc
-# define      __need_wchar_t
-# define      __need_NULL
-#endif
+#define               __need_wchar_t
+#define               __need_NULL
 #include <stddef.h>
Â
 __BEGIN_DECLS
Â
-#ifndef __need_malloc_and_calloc
 #define _STDLIB_H      1
Â
 #if (defined __USE_XOPEN || defined __USE_XOPEN2K8) && !defined _SYS_WAIT_H
@@ -434,10 +431,6 @@ extern int lcong48_r (unsigned short int __param[7],
 # endif /* Use misc. */
 #endif /* Use misc or X/Open. */
Â
-#endif /* don't just need malloc and calloc */
-
-#ifndef __malloc_and_calloc_defined
-# define __malloc_and_calloc_defined
 __BEGIN_NAMESPACE_STD
 /* Allocate SIZE bytes of memory. */
 extern void *malloc (size_t __size) __THROW __attribute_malloc__ __wur;
@@ -445,9 +438,7 @@ extern void *malloc (size_t __size) __THROW __attribute_malloc__ __wur;
 extern void *calloc (size_t __nmemb, size_t __size)
     __THROW __attribute_malloc__ __wur;
 __END_NAMESPACE_STD
-#endif
Â
-#ifndef __need_malloc_and_calloc
 __BEGIN_NAMESPACE_STD
 /* Re-allocate the previously allocated block
   in PTR, making the new block SIZE bytes long. */
@@ -944,9 +935,6 @@ extern int ttyslot (void) __THROW;
 # include <bits/stdlib-ldbl.h>
 #endif
Â
-#endif /* don't just need malloc and calloc */
-#undef __need_malloc_and_calloc
-
 __END_DECLS
Â
 #endif /* stdlib.h */
@@ -118,58 +118,6 @@
 #endif
Â
Â
-/* We need the memory allocation functions for inline strdup().
-Â Â Referring to stdlib.h (even minimally) is not allowed
-  in any of the tight standards compliant modes. */
-#ifdef __USE_MISC
-
-# define __need_malloc_and_calloc
-# include <stdlib.h>
-
-extern char *__strdup (const char *__string) __THROW __attribute_malloc__;
-# define __strdup(s) \
-Â (__extension__ (__builtin_constant_p (s) && __string2_1bptr_p (s)Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ? (((const char *) (s))[0] == '\0'Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ? (char *) calloc ((size_t) 1, (size_t) 1)Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â : ({ size_t __len = strlen (s) + 1;Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â char *__retval = (char *) malloc (__len);Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (__retval != NULL)Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â __retval = (char *) memcpy (__retval, s, __len);Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â __retval; }))Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â : __strdup (s)))
-
-# if defined __USE_XOPEN_EXTENDED || defined __USE_XOPEN2K8
-#Â define strdup(s) __strdup (s)
-# endif
-
-
-extern char *__strndup (const char *__string, size_t __n)
-Â Â Â Â __THROW __attribute_malloc__;
-# define __strndup(s, n) \
-Â (__extension__ (__builtin_constant_p (s) && __string2_1bptr_p (s)Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ? (((const char *) (s))[0] == '\0'Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ? (char *) calloc ((size_t) 1, (size_t) 1)Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â : ({ size_t __len = strlen (s) + 1;Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â size_t __n = (n);Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â char *__retval;Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (__n < __len)Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â __len = __n + 1;Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â __retval = (char *) malloc (__len);Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (__retval != NULL)Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â {Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â __retval[__len - 1] = '\0';Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-                            __retval = (char *) memcpy (__retval, s,        \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â __len - 1);Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â }Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â __retval; }))Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â \
-Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â : __strndup (s, n)))
-
-# ifdef __USE_XOPEN2K8
-#Â define strndup(s, n) __strndup (s, n)
-# endif
-
-#endif /* Use misc. or use GNU. */
-
 #ifndef _FORCE_INLINES
 # undef __STRING_INLINE
 #endif
@@ -636,6 +636,15 @@ extern char *basename (const char *__filename) __THROW __nonnull ((1));
 # endif
 #endif
Â
+#if (defined __USE_XOPEN_EXTENDED || defined __USE_XOPEN2K8Â Â Â Â \
+Â Â Â Â || __GLIBC_USE (LIB_EXT2))
+# define strdup(s) __builtin_strdup (s)
+#endif
+
+#if defined __USE_XOPEN2K8 || __GLIBC_USE (LIB_EXT2)
+#define strndup(s,n) __builtin_strndup (s, n)
+#endif
+
 #if defined __USE_GNU && defined __OPTIMIZE__ \
    && defined __extern_always_inline && __GNUC_PREREQ (3,2)
 # if !defined _FORCE_INLINES && !defined _HAVE_STRING_ARCH_mempcpy