Re: [2.24 PATCH 3/3] Deprecate inclusion of <sys/sysmacros.h> by <sys/types.h>

[Carlos O'Donell <carlos at redhat dot com>]

On 07/31/2016 04:09 PM, Zack Weinberg wrote:
> The macros defined by <sys/sysmacros.h> are not part of POSIX nor XSI, and
> their names frequently collide with user code; see for instance glibc bug
> 19239 and Red Hat bug 130601.  <stdlib.h> includes <sys/types.h> under
> _GNU_SOURCE, and C++ code presently cannot avoid being compiled under
> _GNU_SOURCE, exacerbating the problem.
> 
> ChangeLog:
> 	* NEWS: Inclusion of <sys/sysmacros.h> by <sys/types.h> is deprecated.
> 	* misc/sys/sysmacros.h: If __SYSMACROS_DEPRECATED_INCLUSION is defined,
> 	define major, minor, and makedev to issue deprecation warnings on use.
> 	If __SYSMACROS_DEPRECATED_INCLUSION is *not* defined, suppress
> 	previously-activated deprecation warnings for these macros and prevent
> 	subsequent inclusions of this header from having any effect.
> 	* posix/sys/types.h: Define __SYSMACROS_DEPRECATED_INCLUSION before
> 	including <sys/sysmacros.h>, and undefine it again afterward.

This deprecation looks good.

However, as noted we'll commit this in 2.25.

I will do it for you.

> ---
>  NEWS                 | 16 ++++++++++++++
>  misc/sys/sysmacros.h | 62 +++++++++++++++++++++++++++++++++++++++++++++++++---
>  posix/sys/types.h    |  8 ++++++-
>  3 files changed, 82 insertions(+), 4 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index e2737d5..f57cc99 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -47,6 +47,22 @@ Version 2.24
>    direction of negative infinity.  These are currently enabled as GNU
>    extensions.
>  
> +* The inclusion of <sys/sysmacros.h> by <sys/types.h> is deprecated.  This
> +  means that in a future release, the macros “major”, “minor”, and “makedev”
> +  will only be available from <sys/sysmacros.h>.
> +
> +  These macros are not part of POSIX nor XSI, and their names frequently
> +  collide with user code; see for instance glibc bug 19239 and Red Hat bug
> +  130601.  <stdlib.h> includes <sys/types.h> under _GNU_SOURCE, and C++ code
> +  presently cannot avoid being compiled under _GNU_SOURCE, exacerbating the
> +  problem.
> +

Up to here is OK.

> +  Code that does not need these macros should #undef them after including
> +  <sys/types.h>; this will also improve portability to BSD-derived systems,
> +  where these macros are unconditionally defined by <sys/types.h>.  Code
> +  that *does* need these macros should include <sys/types.h>, and then
> +  include <sys/sysmacros.h> if __GLIBC__ is defined.

I think this should be removed from the NEWS entry.

Instead in the release wiki there should be some detailed instructions
under the normal "Packaging Changes" that describes exactly what needs to be
done with a code snippet, with particular case to writing code that keeps
working into the future.

> +
>  Security related changes:
>  
>  * An unnecessary stack copy in _nss_dns_getnetbyname_r was removed.  It
> diff --git a/misc/sys/sysmacros.h b/misc/sys/sysmacros.h
> index dc2eb83..086e9af 100644
> --- a/misc/sys/sysmacros.h
> +++ b/misc/sys/sysmacros.h
> @@ -16,6 +16,23 @@
>     License along with the GNU C Library; if not, see
>     <http://www.gnu.org/licenses/>.  */
>  
> +#ifndef _SYS_SYSMACROS_H_OUTER
> +
> +#ifndef __SYSMACROS_DEPRECATED_INCLUSION
> +# define _SYS_SYSMACROS_H_OUTER 1
> +#endif
> +
> +/* If <sys/sysmacros.h> is included after <sys/types.h>, these macros
> +   will already be defined, and we need to redefine them without the
> +   deprecation warnings.  (If they are included in the opposite order,
> +   the outer #ifndef will suppress this entire file and the macros
> +   will be usable without warnings.)  */
> +#undef major
> +#undef minor
> +#undef makedev
> +
> +/* This is the macro that must be defined to satisfy the misuse check
> +   in bits/sysmacros.h. */
>  #ifndef _SYS_SYSMACROS_H
>  #define _SYS_SYSMACROS_H 1
>  
> @@ -23,32 +40,65 @@
>  #include <bits/types.h>
>  #include <bits/sysmacros.h>
>  
> +/* The extra "\n " moves gcc's [-Wdeprecated-declarations] annotation
> +   onto the next line.  */
> +#define __SYSMACROS_DEPRECATION_MSG(symbol)				     \
> +  "\n  In the GNU C Library, `" #symbol "' is defined by <sys/sysmacros.h>." \
> +  "\n  For historical compatibility, it is currently defined by"	     \
> +  "\n  <sys/types.h> as well, but we plan to remove this soon."		     \
> +  "\n  To use `" #symbol "', include <sys/sysmacros.h> directly."	     \
> +  "\n  If you did not intend to use a system-defined macro `" #symbol "',"   \
> +  "\n  you should #undef it after including <sys/types.h>."		     \
> +  "\n "
> +

OK.

>  #define __SYSMACROS_DECL_TEMPL(rtype, name, proto)			     \
>    extern rtype gnu_dev_##name proto __THROW __attribute_const__;
>  
> +#define __SYSMACROS_FST_DECL_TEMPL(rtype, name, proto)			     \
> +  extern rtype __REDIRECT_NTH (__##name##_from_sys_types, proto,	     \
> +			       gnu_dev_##name)				     \
> +       __attribute_const__						     \
> +       __attribute_deprecated_msg__ (__SYSMACROS_DEPRECATION_MSG (name));
> +

OK.

>  #define __SYSMACROS_IMPL_TEMPL(rtype, name, proto)			     \
>    __extension__ __extern_inline __attribute_const__ rtype		     \
>    __NTH (gnu_dev_##name proto)
>  
> +#define __SYSMACROS_FST_IMPL_TEMPL(rtype, name, proto)			     \
> +  __extension__ __extern_inline __attribute_const__ rtype		     \
> +  __NTH (__##name##_from_sys_types proto)
> +

OK.

>  __BEGIN_DECLS
>  
>  __SYSMACROS_DECLARE_MAJOR (__SYSMACROS_DECL_TEMPL)
>  __SYSMACROS_DECLARE_MINOR (__SYSMACROS_DECL_TEMPL)
>  __SYSMACROS_DECLARE_MAKEDEV (__SYSMACROS_DECL_TEMPL)
>  
> +__SYSMACROS_DECLARE_MAJOR (__SYSMACROS_FST_DECL_TEMPL)
> +__SYSMACROS_DECLARE_MINOR (__SYSMACROS_FST_DECL_TEMPL)
> +__SYSMACROS_DECLARE_MAKEDEV (__SYSMACROS_FST_DECL_TEMPL)
> +

OK.

>  #ifdef __USE_EXTERN_INLINES
>  
>  __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_IMPL_TEMPL)
>  __SYSMACROS_DEFINE_MINOR (__SYSMACROS_IMPL_TEMPL)
>  __SYSMACROS_DEFINE_MAKEDEV (__SYSMACROS_IMPL_TEMPL)
>  
> +__SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL)
> +__SYSMACROS_DEFINE_MINOR (__SYSMACROS_FST_IMPL_TEMPL)
> +__SYSMACROS_DEFINE_MAKEDEV (__SYSMACROS_FST_IMPL_TEMPL)
> +

OK.

>  #endif
>  
>  __END_DECLS
>  
> +#endif /* _SYS_SYSMACROS_H */
> +
>  #ifndef __SYSMACROS_NEED_IMPLEMENTATION
>  # undef __SYSMACROS_DECL_TEMPL
> +# undef __SYSMACROS_FST_DECL_TEMPL
>  # undef __SYSMACROS_IMPL_TEMPL
> +# undef __SYSMACROS_FST_IMPL_TEMPL
>  # undef __SYSMACROS_DECLARE_MAJOR
>  # undef __SYSMACROS_DECLARE_MINOR
>  # undef __SYSMACROS_DECLARE_MAKEDEV
> @@ -57,8 +107,14 @@ __END_DECLS
>  # undef __SYSMACROS_DEFINE_MAKEDEV
>  #endif
>  
> -#define major(dev) gnu_dev_major (dev)
> -#define minor(dev) gnu_dev_minor (dev)
> -#define makedev(maj, min) gnu_dev_makedev (maj, min)
> +#ifdef __SYSMACROS_DEPRECATED_INCLUSION
> +# define major(dev) __major_from_sys_types (dev)
> +# define minor(dev) __minor_from_sys_types (dev)
> +# define makedev(maj, min) __makedev_from_sys_types (maj, min)
> +#else
> +# define major(dev) gnu_dev_major (dev)
> +# define minor(dev) gnu_dev_minor (dev)
> +# define makedev(maj, min) gnu_dev_makedev (maj, min)
> +#endif
>  

OK.

>  #endif /* sys/sysmacros.h */
> diff --git a/posix/sys/types.h b/posix/sys/types.h
> index a728567..83dadcd 100644
> --- a/posix/sys/types.h
> +++ b/posix/sys/types.h
> @@ -218,8 +218,14 @@ typedef int register_t __attribute__ ((__mode__ (__word__)));
>  /* It also defines `fd_set' and the FD_* macros for `select'.  */
>  # include <sys/select.h>
>  
> -/* BSD defines these symbols, so we follow.  */
> +/* BSD defines `major', `minor', and `makedev' in this header.
> +   However, these symbols are likely to collide with user code, so we are
> +   going to stop defining them here in an upcoming release.  Code that needs
> +   these macros should include <sys/sysmacros.h> directly.  Code that does
> +   not need these macros should #undef them after including this header.  */

OK.

> +# define __SYSMACROS_DEPRECATED_INCLUSION
>  # include <sys/sysmacros.h>
> +# undef __SYSMACROS_DEPRECATED_INCLUSION
>  #endif /* Use misc.  */
>  
>  
> 


-- 
Cheers,
Carlos.