This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [GLIBC][PATCH v2] Remove strdup inlines


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
> 
> 
>     
> 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]