Re: RFC: deprecate sys/sysmacros.h inclusion from sys/types.h

[Roland McGrath <roland at hack dot frob dot com>]

This patch has no ChangeLog entries.

Please drop the clang-related changes to sys/cdefs.h in this patch.
Adding __attribute_deprecated_msg__ here is OK, but other changes to
sys/cdefs.h belong in a separate change.

> --- /dev/null
> +++ b/include/sys/sysmacros.h
> @@ -0,0 +1,3 @@
> +#ifndef _SYS_SYSMACROS_H
> +#include <misc/sys/sysmacros.h>
> +#endif

No #ifndef here.  include/ files should be nothing but direct
wrappers when that works, as it should here.

> --- /dev/null
> +++ b/misc/makedev.c

Moving this file to OS-independent place means you also need to add
the functions to misc/Versions.  Since they didn't exist before on
non-Linux configurations, misc/Versions should list them under
GLIBC_2.24.  The existing sysdeps/unix/sysv/linux/Versions entries
will give them the old symbol version in Linux configurations.

> --- /dev/null
> +++ b/misc/sys/sysmacros.h

Throughout this file you should indent nested preprocessor
directives, e.g.:

#ifdef foo
# define bar ...
#endif


> +#define __INCLUSION_DEPRECATION_MSG(symbol)                                  \

Use a name that starts with "__SYSMACROS_".

> +  "\n  The macro `" #symbol "' is defined by <sys/sysmacros.h>."             \
> +  "\n  For compatibility with BSD, it is currently defined by <sys/types.h>" \
> +  "\n  as well, but we plan to remove this soon.  To use `" #symbol "',"     \
> +  "\n  include <sys/sysmacros.h> directly.  If you did not intend to use"    \
> +  "\n  a system-defined macro `" #symbol "', you can suppress it by "        \
> +  "\n  defining the macro _SYS_TYPES_NO_SYSMACROS (with any value) before "  \
> +  "\n  including any system headers."

Say "historical compatibility" rather than "compatibility with BSD".

I'm not convinced that we should support the _SYS_TYPES_NO_SYSMACROS
macro at all.  If we advise applications to define that, then it will
litter random sources for years to come.  But it serves only a brief
transitional purpose.  And IMHO there is really no need for that
option at all.  Things will continue to work as they did today with
no warnings for applications that were not using these symbols.

> +#ifdef __USE_EXTERN_INLINES
> +#define __SYSMACRO_IMPL(rtype, name, proto, expr) \

An example of unintended preprocessor directives.

> +  __SYSMACRO_DECL(rtype, name, proto) \

Space before paren here.

> +__SYSMACRO_IMPL(unsigned int, major, (__dev_t __dev), __dev_major (__dev))
> +__SYSMACRO_IMPL(unsigned int, minor, (__dev_t __dev), __dev_minor (__dev))
> +__SYSMACRO_IMPL(__dev_t, makedev,
> +                (unsigned int __major, unsigned int __minor),
> +                __dev_makedev (__major, __minor))

Space before parens here.

Also, make all these macros __SYSMACROS_* instead of _SYSMACRO_* (so
the prefix matches the name of the header file).

> +#ifdef _DEPRECATED_INCLUSION_OF_SYS_SYSMACROS_H
> +#define major(dev) gnu_dev_major_from_sys_types (dev)
> +#define minor(dev) gnu_dev_minor_from_sys_types (dev)
> +#define makedev(maj, min) gnu_dev_makedev_from_sys_types (maj, min)

I think these compat wrapper functions should have __ names.


Thanks,
Roland