Use SHLIB_COMPAT for libc symbols only if building libc.so

  On Tue, Mar 18, 2014 at 03:48:14PM -0700, Roland McGrath wrote:
> This seems worse to me.  I've been vaguely noodling about some centralized
> macro regime to make the NOT_IN_*/IS_IN_* cases #if-friendly.  Let's figure
> out what we want to do about those in general before considering
> SHLIB_COMPAT.  I tend to think that it's a more maintainable interface to
> have '#if SHLIB_COMPAT (libfoo, ...)' be a standalone test that encompasses
> the IS_IN_libfoo test (just as it encompasses the SHARED test).

The NOT_IN_libc definition (non-definition actually) is only used to
define IS_IN_libc in cases where there is no other IS_IN_*.  This can
be trivially used to set IS_IN_libc to 0 when NOT_IN_libc is set,
giving us the #if-friendly definition we want.

There is only one place (other than SHLIB_COMPAT) where IS_IN_libc is
used directly and I've fixed that up to use #if instead of #ifdef.

A further enhancement here would be to replace all uses of #ifdef
NOT_IN_libc with #if !IS_IN_libc so that NOT_IN_libc has just this one
specific usage.

I have avoided a warning on 'SHARED' by using ifdef on purpose for now
since it can be changed later if needed.

The generated code is identical before and after this change on
x86_64.

Siddhesh

	* include/shlib-compat.h: Define IS_IN_libc early.
	[NOT_IN_libc]: Define IS_IN_libc to 0.
	* nss/nss_files/files-parse.c: Include shlib-compat.h.
	[!ENTDATA]: Check for value of IS_IN_libc and not just if it
	is defined.
  

Ping!

On Mon, Jun 30, 2014 at 04:56:44PM +0530, Siddhesh Poyarekar wrote:
> On Tue, Mar 18, 2014 at 03:48:14PM -0700, Roland McGrath wrote:
> > This seems worse to me.  I've been vaguely noodling about some centralized
> > macro regime to make the NOT_IN_*/IS_IN_* cases #if-friendly.  Let's figure
> > out what we want to do about those in general before considering
> > SHLIB_COMPAT.  I tend to think that it's a more maintainable interface to
> > have '#if SHLIB_COMPAT (libfoo, ...)' be a standalone test that encompasses
> > the IS_IN_libfoo test (just as it encompasses the SHARED test).
> 
> The NOT_IN_libc definition (non-definition actually) is only used to
> define IS_IN_libc in cases where there is no other IS_IN_*.  This can
> be trivially used to set IS_IN_libc to 0 when NOT_IN_libc is set,
> giving us the #if-friendly definition we want.
> 
> There is only one place (other than SHLIB_COMPAT) where IS_IN_libc is
> used directly and I've fixed that up to use #if instead of #ifdef.
> 
> A further enhancement here would be to replace all uses of #ifdef
> NOT_IN_libc with #if !IS_IN_libc so that NOT_IN_libc has just this one
> specific usage.
> 
> I have avoided a warning on 'SHARED' by using ifdef on purpose for now
> since it can be changed later if needed.
> 
> The generated code is identical before and after this change on
> x86_64.
> 
> Siddhesh
> 
> 	* include/shlib-compat.h: Define IS_IN_libc early.
> 	[NOT_IN_libc]: Define IS_IN_libc to 0.
> 	* nss/nss_files/files-parse.c: Include shlib-compat.h.
> 	[!ENTDATA]: Check for value of IS_IN_libc and not just if it
> 	is defined.
> 
> diff --git a/include/shlib-compat.h b/include/shlib-compat.h
> index fac0814..3b6a227 100644
> --- a/include/shlib-compat.h
> +++ b/include/shlib-compat.h
> @@ -23,6 +23,12 @@
>  
>  # include <abi-versions.h>
>  
> +# ifndef NOT_IN_libc
> +#  define IS_IN_libc 1
> +# else
> +#  define IS_IN_libc 0
> +# endif
> +
>  /* The file abi-versions.h (generated by scripts/abi-versions.awk) defines
>     symbols like `ABI_libm_GLIBC_2_0' for each version set in the source
>     code for each library.  For a version set that is subsumed by a later
> @@ -45,10 +51,6 @@
>     && (!(ABI_##lib##_##obsoleted - 0)					      \
>         || ((ABI_##lib##_##introduced - 0) < (ABI_##lib##_##obsoleted - 0))))
>  
> -# ifndef NOT_IN_libc
> -#  define IS_IN_libc 1
> -# endif
> -
>  /* That header also defines symbols like `VERSION_libm_GLIBC_2_1' to
>     the version set name to use for e.g. symbols first introduced into
>     libm in the GLIBC_2.1 version.  Definitions of symbols with explicit
> diff --git a/nss/nss_files/files-parse.c b/nss/nss_files/files-parse.c
> index 1da1a6f..35a232c 100644
> --- a/nss/nss_files/files-parse.c
> +++ b/nss/nss_files/files-parse.c
> @@ -21,6 +21,7 @@
>  #include <string.h>
>  #include <stdlib.h>
>  #include <stdint.h>
> +#include <shlib-compat.h>
>  
>  /* These symbols are defined by the including source file:
>  
> @@ -73,12 +74,12 @@ struct parser_data
>  /* Export the line parser function so it can be used in nss_db.  */
>  # define parser_stclass /* Global */
>  # define parse_line CONCAT(_nss_files_parse_,ENTNAME)
> -# ifdef IS_IN_libc
> +# if defined SHARED && !IS_IN_libc
> +#  define nss_files_parse_hidden_def(name) libnss_files_hidden_def (name)
> +# else
>  /* We are defining one of the functions that actually lives in libc
>     because it is used to implement fget*ent and suchlike.  */
> -#  define nss_files_parse_hidden_def(name) libc_hidden_def (name)
> -# else
> -#  define nss_files_parse_hidden_def(name) libnss_files_hidden_def (name)
> +#   define nss_files_parse_hidden_def(name) libc_hidden_def (name)
>  # endif
>  #endif
>
  

Ping!

On Tue, Jul 08, 2014 at 03:11:06PM +0530, Siddhesh Poyarekar wrote:
> Ping!
> 
> On Mon, Jun 30, 2014 at 04:56:44PM +0530, Siddhesh Poyarekar wrote:
> > On Tue, Mar 18, 2014 at 03:48:14PM -0700, Roland McGrath wrote:
> > > This seems worse to me.  I've been vaguely noodling about some centralized
> > > macro regime to make the NOT_IN_*/IS_IN_* cases #if-friendly.  Let's figure
> > > out what we want to do about those in general before considering
> > > SHLIB_COMPAT.  I tend to think that it's a more maintainable interface to
> > > have '#if SHLIB_COMPAT (libfoo, ...)' be a standalone test that encompasses
> > > the IS_IN_libfoo test (just as it encompasses the SHARED test).
> > 
> > The NOT_IN_libc definition (non-definition actually) is only used to
> > define IS_IN_libc in cases where there is no other IS_IN_*.  This can
> > be trivially used to set IS_IN_libc to 0 when NOT_IN_libc is set,
> > giving us the #if-friendly definition we want.
> > 
> > There is only one place (other than SHLIB_COMPAT) where IS_IN_libc is
> > used directly and I've fixed that up to use #if instead of #ifdef.
> > 
> > A further enhancement here would be to replace all uses of #ifdef
> > NOT_IN_libc with #if !IS_IN_libc so that NOT_IN_libc has just this one
> > specific usage.
> > 
> > I have avoided a warning on 'SHARED' by using ifdef on purpose for now
> > since it can be changed later if needed.
> > 
> > The generated code is identical before and after this change on
> > x86_64.
> > 
> > Siddhesh
> > 
> > 	* include/shlib-compat.h: Define IS_IN_libc early.
> > 	[NOT_IN_libc]: Define IS_IN_libc to 0.
> > 	* nss/nss_files/files-parse.c: Include shlib-compat.h.
> > 	[!ENTDATA]: Check for value of IS_IN_libc and not just if it
> > 	is defined.
> > 
> > diff --git a/include/shlib-compat.h b/include/shlib-compat.h
> > index fac0814..3b6a227 100644
> > --- a/include/shlib-compat.h
> > +++ b/include/shlib-compat.h
> > @@ -23,6 +23,12 @@
> >  
> >  # include <abi-versions.h>
> >  
> > +# ifndef NOT_IN_libc
> > +#  define IS_IN_libc 1
> > +# else
> > +#  define IS_IN_libc 0
> > +# endif
> > +
> >  /* The file abi-versions.h (generated by scripts/abi-versions.awk) defines
> >     symbols like `ABI_libm_GLIBC_2_0' for each version set in the source
> >     code for each library.  For a version set that is subsumed by a later
> > @@ -45,10 +51,6 @@
> >     && (!(ABI_##lib##_##obsoleted - 0)					      \
> >         || ((ABI_##lib##_##introduced - 0) < (ABI_##lib##_##obsoleted - 0))))
> >  
> > -# ifndef NOT_IN_libc
> > -#  define IS_IN_libc 1
> > -# endif
> > -
> >  /* That header also defines symbols like `VERSION_libm_GLIBC_2_1' to
> >     the version set name to use for e.g. symbols first introduced into
> >     libm in the GLIBC_2.1 version.  Definitions of symbols with explicit
> > diff --git a/nss/nss_files/files-parse.c b/nss/nss_files/files-parse.c
> > index 1da1a6f..35a232c 100644
> > --- a/nss/nss_files/files-parse.c
> > +++ b/nss/nss_files/files-parse.c
> > @@ -21,6 +21,7 @@
> >  #include <string.h>
> >  #include <stdlib.h>
> >  #include <stdint.h>
> > +#include <shlib-compat.h>
> >  
> >  /* These symbols are defined by the including source file:
> >  
> > @@ -73,12 +74,12 @@ struct parser_data
> >  /* Export the line parser function so it can be used in nss_db.  */
> >  # define parser_stclass /* Global */
> >  # define parse_line CONCAT(_nss_files_parse_,ENTNAME)
> > -# ifdef IS_IN_libc
> > +# if defined SHARED && !IS_IN_libc
> > +#  define nss_files_parse_hidden_def(name) libnss_files_hidden_def (name)
> > +# else
> >  /* We are defining one of the functions that actually lives in libc
> >     because it is used to implement fget*ent and suchlike.  */
> > -#  define nss_files_parse_hidden_def(name) libc_hidden_def (name)
> > -# else
> > -#  define nss_files_parse_hidden_def(name) libnss_files_hidden_def (name)
> > +#   define nss_files_parse_hidden_def(name) libc_hidden_def (name)
> >  # endif
> >  #endif
> >  
> 
>