public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [2.24 PATCH 2/3] Minimize sysdeps code involved in defining major/minor/makedev.
  2016-07-31 20:09 [2.24 PATCH 0/3] sys/sysmacros.h and sys.types.h, revised^n Zack Weinberg
@ 2016-07-31 20:09 ` Zack Weinberg
  2016-08-01 20:55   ` Carlos O'Donell
  2016-07-31 20:09 ` [2.24 PATCH 3/3] Deprecate inclusion of <sys/sysmacros.h> by <sys/types.h> Zack Weinberg
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Zack Weinberg @ 2016-07-31 20:09 UTC (permalink / raw)
  To: libc-alpha; +Cc: adhemerval.zanella, vapier, carlos

Presently sys/sysmacros.h is entirely defined in sysdeps.  This would
mean that the deprecation logic coming up in the next patch would have
to be written twice (in generic/ and unix/sysv/linux/).  To avoid that,
hoist all but the unavoidably system-dependent logic to misc/, leaving a
bits/ header behind.  This also promotes the Linux-specific encoding of
dev_t, which accommodates 32-bit major and minor numbers in a 64-bit dev_t,
to generic, as glibc's dev_t is always 64 bits wide.

The former Linux implementation used inline functions to avoid evaluating
arguments more than once.  After this change, all platforms use inline
functions, which means that three new symbols are added to the generic ABI.
Since NaCl is the only non-Linux port currently in the tree, the only
libc.abilist file that requires a change is arm/nacl/libc.abilist.

New ports henceforth need only provide a bits/sysmacros.h defining
internal macros __SYSMACROS_{DECLARE,DEFINE}_{MAJOR,MINOR,MAKEDEV}.

While I was at it, I added a basic round-trip test for these functions.

ChangeLog:

	* sysdeps/generic/sys/sysmacros.h: Delete file.
	* sysdeps/unix/sysv/linux/makedev.c: Delete file.
	* sysdeps/unix/sysv/linux/sys/sysmacros.h: Move file ...
	* bits/sysmacros.h: ... here; this encoding is now the generic
	encoding.  Now defines only the following macros:
	__SYSMACROS_DECLARE_MAJOR, __SYSMACROS_DEFINE_MAJOR,
	__SYSMACROS_DECLARE_MINOR, __SYSMACROS_DEFINE_MINOR,
	__SYSMACROS_DECLARE_MAKEDEV, __SYSMACROS_DEFINE_MAKEDEV.

	* misc/sys/sysmacros.h, misc/makedev.c: New files that use
	bits/sysmacros.h and the above new macros to generate the
	public implementations of major, minor, and makedev.
	* misc/tst-makedev.c: New test.
	* include/sys/sysmacros.h: New wrapper.

	* misc/Makefile (headers): Add sys/sysmacros.h, bits/sysmacros.h.
	(routines): Add makedev.
	(tests): Add tst-makedev.
	* misc/Versions [GLIBC_2.24]: Add gnu_dev_major, gnu_dev_minor,
	gnu_dev_makedev.
	* posix/Makefile (headers): Remove sys/sysmacros.h.
	* sysdeps/unix/sysv/linux/Makefile (sysdep_routines): Remove makedev.

	* sysdeps/arm/nacl/libc.abilist: Add GLIBC_2.24,
	gnu_dev_major, gnu_dev_makedev, gnu_dev_minor.
---
 bits/sysmacros.h                        |  74 +++++++++++++++++++++++
 include/sys/sysmacros.h                 |   1 +
 misc/Makefile                           |   7 ++-
 misc/Versions                           |   3 +
 misc/makedev.c                          |  30 +++++++++
 misc/sys/sysmacros.h                    |  64 ++++++++++++++++++++
 misc/tst-makedev.c                      | 104 ++++++++++++++++++++++++++++++++
 posix/Makefile                          |   2 +-
 sysdeps/arm/nacl/libc.abilist           |   4 ++
 sysdeps/generic/sys/sysmacros.h         |  30 ---------
 sysdeps/unix/sysv/linux/Makefile        |   2 +-
 sysdeps/unix/sysv/linux/makedev.c       |  40 ------------
 sysdeps/unix/sysv/linux/sys/sysmacros.h |  65 --------------------
 13 files changed, 286 insertions(+), 140 deletions(-)
 create mode 100644 bits/sysmacros.h
 create mode 100644 include/sys/sysmacros.h
 create mode 100644 misc/makedev.c
 create mode 100644 misc/sys/sysmacros.h
 create mode 100644 misc/tst-makedev.c
 delete mode 100644 sysdeps/generic/sys/sysmacros.h
 delete mode 100644 sysdeps/unix/sysv/linux/makedev.c
 delete mode 100644 sysdeps/unix/sysv/linux/sys/sysmacros.h

diff --git a/bits/sysmacros.h b/bits/sysmacros.h
new file mode 100644
index 0000000..e2c0bc2
--- /dev/null
+++ b/bits/sysmacros.h
@@ -0,0 +1,74 @@
+/* Definitions of macros to access `dev_t' values.
+   Copyright (C) 1996-2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _BITS_SYSMACROS_H
+#define _BITS_SYSMACROS_H 1
+
+#ifndef _SYS_SYSMACROS_H
+# error "Never include <bits/sysmacros.h> directly; use <sys/sysmacros.h> instead."
+#endif
+
+/* dev_t in glibc is a 64-bit quantity, with 32-bit major and minor numbers.
+   Our default encoding is MMMM Mmmm mmmM MMmm, where M is a hex digit of
+   the major number and m is a hex digit of the minor number.  This is
+   downward compatible with legacy systems where dev_t is 16 bits wide,
+   encoded as MMmm.  It is also downward compatible with the Linux kernel,
+   which (as of 2016) uses 32-bit dev_t, encoded as mmmM MMmm.
+
+   Systems that use an incompatible encoding for dev_t should override this
+   file in the appropriate sysdeps subdirectory.  */
+
+#define __SYSMACROS_DECLARE_MAJOR(DECL_TEMPL)			\
+  DECL_TEMPL(unsigned int, major, (__dev_t __dev))
+
+#define __SYSMACROS_DEFINE_MAJOR(DECL_TEMPL)			\
+  __SYSMACROS_DECLARE_MAJOR (DECL_TEMPL)			\
+  {								\
+    unsigned int __major;					\
+    __major  = ((__dev & (__dev_t) 0x00000000000fff00u) >>  8); \
+    __major |= ((__dev & (__dev_t) 0xfffff00000000000u) >> 32); \
+    return __major;						\
+  }
+
+#define __SYSMACROS_DECLARE_MINOR(DECL_TEMPL)			\
+  DECL_TEMPL(unsigned int, minor, (__dev_t __dev))
+
+#define __SYSMACROS_DEFINE_MINOR(DECL_TEMPL)			\
+  __SYSMACROS_DECLARE_MINOR (DECL_TEMPL)			\
+  {								\
+    unsigned int __minor;					\
+    __minor  = ((__dev & (__dev_t) 0x00000000000000ffu) >>  0); \
+    __minor |= ((__dev & (__dev_t) 0x00000ffffff00000u) >> 12); \
+    return __minor;						\
+  }
+
+#define __SYSMACROS_DECLARE_MAKEDEV(DECL_TEMPL)			\
+  DECL_TEMPL(__dev_t, makedev, (unsigned int __major, unsigned int __minor))
+
+#define __SYSMACROS_DEFINE_MAKEDEV(DECL_TEMPL)			\
+  __SYSMACROS_DECLARE_MAKEDEV (DECL_TEMPL)			\
+  {								\
+    __dev_t __dev;						\
+    __dev  = (((__dev_t) (__major & 0x00000fffu)) <<  8);	\
+    __dev |= (((__dev_t) (__major & 0xfffff000u)) << 32);	\
+    __dev |= (((__dev_t) (__minor & 0x000000ffu)) <<  0);	\
+    __dev |= (((__dev_t) (__minor & 0xffffff00u)) << 12);	\
+    return __dev;						\
+  }
+
+#endif /* bits/sysmacros.h */
diff --git a/include/sys/sysmacros.h b/include/sys/sysmacros.h
new file mode 100644
index 0000000..87813c5
--- /dev/null
+++ b/include/sys/sysmacros.h
@@ -0,0 +1 @@
+#include <misc/sys/sysmacros.h>
diff --git a/misc/Makefile b/misc/Makefile
index 56e20ce..3d2ebb8 100644
--- a/misc/Makefile
+++ b/misc/Makefile
@@ -34,7 +34,8 @@ headers	:= sys/uio.h bits/uio.h sys/ioctl.h bits/ioctls.h bits/ioctl-types.h \
 	   regexp.h bits/select.h bits/mman.h sys/xattr.h \
 	   syslog.h sys/syslog.h \
 	   bits/syslog.h bits/syslog-ldbl.h bits/syslog-path.h bits/error.h \
-	   bits/select2.h bits/hwcap.h sys/auxv.h
+	   bits/select2.h bits/hwcap.h sys/auxv.h \
+	   sys/sysmacros.h bits/sysmacros.h
 
 routines := brk sbrk sstk ioctl \
 	    readv writev preadv preadv64 pwritev pwritev64 \
@@ -67,7 +68,7 @@ routines := brk sbrk sstk ioctl \
 	    getloadavg getclktck \
 	    fgetxattr flistxattr fremovexattr fsetxattr getxattr \
 	    listxattr lgetxattr llistxattr lremovexattr lsetxattr \
-	    removexattr setxattr getauxval ifunc-impl-list
+	    removexattr setxattr getauxval ifunc-impl-list makedev
 
 generated += tst-error1.mtrace tst-error1-mem.out
 
@@ -78,7 +79,7 @@ gpl2lgpl := error.c error.h
 tests := tst-dirname tst-tsearch tst-fdset tst-efgcvt tst-mntent tst-hsearch \
 	 tst-error1 tst-pselect tst-insremque tst-mntent2 bug-hsearch1 \
 	 tst-mntent-blank-corrupt tst-mntent-blank-passno bug18240 \
-	 tst-preadvwritev tst-preadvwritev64
+	 tst-preadvwritev tst-preadvwritev64 tst-makedev
 ifeq ($(run-built-tests),yes)
 tests-special += $(objpfx)tst-error1-mem.out
 endif
diff --git a/misc/Versions b/misc/Versions
index 671f487..afa22bd 100644
--- a/misc/Versions
+++ b/misc/Versions
@@ -152,6 +152,9 @@ libc {
   GLIBC_2.23 {
     # SHLIB_COMPAT(GLIBC_2_0, GLIBC_2_23) used in regexp.c
   }
+  GLIBC_2.24 {
+    gnu_dev_major; gnu_dev_minor; gnu_dev_makedev;
+  }
   GLIBC_PRIVATE {
     __madvise;
     __mktemp;
diff --git a/misc/makedev.c b/misc/makedev.c
new file mode 100644
index 0000000..7303fb6
--- /dev/null
+++ b/misc/makedev.c
@@ -0,0 +1,30 @@
+/* Definitions of functions to access `dev_t' values.
+   Copyright (C) 2003-2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <features.h>
+
+#undef __USE_EXTERN_INLINES
+#define __SYSMACROS_NEED_IMPLEMENTATION
+#include <sys/sysmacros.h>
+
+#define OUT_OF_LINE_IMPL_TEMPL(rtype, name, proto) \
+  rtype gnu_dev_##name proto
+
+__SYSMACROS_DEFINE_MAJOR(OUT_OF_LINE_IMPL_TEMPL)
+__SYSMACROS_DEFINE_MINOR(OUT_OF_LINE_IMPL_TEMPL)
+__SYSMACROS_DEFINE_MAKEDEV(OUT_OF_LINE_IMPL_TEMPL)
diff --git a/misc/sys/sysmacros.h b/misc/sys/sysmacros.h
new file mode 100644
index 0000000..dc2eb83
--- /dev/null
+++ b/misc/sys/sysmacros.h
@@ -0,0 +1,64 @@
+/* Definitions of macros to access `dev_t' values.
+   Copyright (C) 1996-2015 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _SYS_SYSMACROS_H
+#define _SYS_SYSMACROS_H 1
+
+#include <features.h>
+#include <bits/types.h>
+#include <bits/sysmacros.h>
+
+#define __SYSMACROS_DECL_TEMPL(rtype, name, proto)			     \
+  extern rtype gnu_dev_##name proto __THROW __attribute_const__;
+
+#define __SYSMACROS_IMPL_TEMPL(rtype, name, proto)			     \
+  __extension__ __extern_inline __attribute_const__ rtype		     \
+  __NTH (gnu_dev_##name proto)
+
+__BEGIN_DECLS
+
+__SYSMACROS_DECLARE_MAJOR (__SYSMACROS_DECL_TEMPL)
+__SYSMACROS_DECLARE_MINOR (__SYSMACROS_DECL_TEMPL)
+__SYSMACROS_DECLARE_MAKEDEV (__SYSMACROS_DECL_TEMPL)
+
+#ifdef __USE_EXTERN_INLINES
+
+__SYSMACROS_DEFINE_MAJOR (__SYSMACROS_IMPL_TEMPL)
+__SYSMACROS_DEFINE_MINOR (__SYSMACROS_IMPL_TEMPL)
+__SYSMACROS_DEFINE_MAKEDEV (__SYSMACROS_IMPL_TEMPL)
+
+#endif
+
+__END_DECLS
+
+#ifndef __SYSMACROS_NEED_IMPLEMENTATION
+# undef __SYSMACROS_DECL_TEMPL
+# undef __SYSMACROS_IMPL_TEMPL
+# undef __SYSMACROS_DECLARE_MAJOR
+# undef __SYSMACROS_DECLARE_MINOR
+# undef __SYSMACROS_DECLARE_MAKEDEV
+# undef __SYSMACROS_DEFINE_MAJOR
+# undef __SYSMACROS_DEFINE_MINOR
+# 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)
+
+#endif /* sys/sysmacros.h */
diff --git a/misc/tst-makedev.c b/misc/tst-makedev.c
new file mode 100644
index 0000000..b330b5a
--- /dev/null
+++ b/misc/tst-makedev.c
@@ -0,0 +1,104 @@
+/* Tests of functions to access `dev_t' values.
+   Copyright (C) 2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <sys/types.h>
+#include <sys/sysmacros.h>
+#include <stdio.h>
+#include <inttypes.h>
+
+/* Confirm that makedev (major (d), minor (d)) == d.  */
+static int
+do_test_split_combine (dev_t d1)
+{
+  unsigned int maj = major (d1);
+  unsigned int min = minor (d1);
+  dev_t d2 = makedev (maj, min);
+  if (d1 != d2)
+    {
+      printf ("FAIL: %016" PRIx64 " != %016" PRIx64 " (maj %08x min %08x)\n",
+	      (uint64_t)d2, (uint64_t)d1, maj, min);
+      return 1;
+    }
+  else
+    return 0;
+}
+
+/* Confirm that major (makedev (maj, min)) == maj and
+   minor (makedev (maj, min)) == min.  */
+static int
+do_test_combine_split (unsigned int maj1, unsigned int min1)
+{
+  dev_t d = makedev (maj1, min1);
+  unsigned int maj2 = major (d);
+  unsigned int min2 = minor (d);
+  if (maj1 != maj2 && min1 != min2)
+    {
+      printf ("FAIL: %08x != %08x, %08x != %08x (dev %016" PRIx64 ")\n",
+	      maj2, maj1, min2, min1, (uint64_t)d);
+      return 1;
+    }
+  else if (maj1 != maj2)
+    {
+      printf ("FAIL: %08x != %08x, %08x == %08x (dev %016" PRIx64 ")\n",
+	      maj2, maj1, min2, min1, (uint64_t)d);
+      return 1;
+    }
+  else if (min1 != min2)
+    {
+      printf ("FAIL: %08x == %08x, %08x != %08x (dev %016" PRIx64 ")\n",
+	      maj2, maj1, min2, min1, (uint64_t)d);
+      return 1;
+    }
+  else
+    return 0;
+}
+
+static int
+do_test (void)
+{
+  dev_t d;
+  unsigned int maj, min;
+  int status = 0;
+
+  /* Test the traditional range (16-bit dev_t, 8-bit each maj/min)
+     exhaustively.  */
+  for (d = 0; d <= 0xFFFF; d++)
+    status |= do_test_split_combine (d);
+
+  for (maj = 0; maj <= 0xFF; maj++)
+    for (min = 0; min <= 0xFF; min++)
+      status |= do_test_combine_split (maj, min);
+
+  /* Test glibc's expanded range (64-bit dev_t, 32-bit each maj/min).
+     Exhaustive testing would take much too long, instead we shift a
+     pair of 1-bits over each range.  */
+  {
+    unsigned int a, b;
+    for (a = 0; a <= 63; a++)
+      do_test_split_combine (((dev_t) 0x03) << a);
+
+    for (a = 0; a < 31; a++)
+      for (b = 0; b <= 31; b++)
+	do_test_combine_split (0x03u << a, 0x03u << b);
+  }
+
+  return status;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/posix/Makefile b/posix/Makefile
index 5b0e298..3a7719e 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -29,7 +29,7 @@ headers	:= sys/utsname.h sys/times.h sys/wait.h sys/types.h unistd.h	      \
 	   bits/local_lim.h tar.h bits/utsname.h bits/confname.h	      \
 	   bits/waitflags.h bits/waitstatus.h sys/unistd.h sched.h	      \
 	   bits/sched.h re_comp.h wait.h bits/environments.h cpio.h	      \
-	   sys/sysmacros.h spawn.h bits/unistd.h
+	   spawn.h bits/unistd.h
 
 routines :=								      \
 	uname								      \
diff --git a/sysdeps/arm/nacl/libc.abilist b/sysdeps/arm/nacl/libc.abilist
index 2f7751d..87b7139 100644
--- a/sysdeps/arm/nacl/libc.abilist
+++ b/sysdeps/arm/nacl/libc.abilist
@@ -1840,4 +1840,8 @@ GLIBC_2.23 fts64_close F
 GLIBC_2.23 fts64_open F
 GLIBC_2.23 fts64_read F
 GLIBC_2.23 fts64_set F
+GLIBC_2.24 GLIBC_2.24 A
 GLIBC_2.24 quick_exit F
+GLIBC_2.24 gnu_dev_major F
+GLIBC_2.24 gnu_dev_makedev F
+GLIBC_2.24 gnu_dev_minor F
diff --git a/sysdeps/generic/sys/sysmacros.h b/sysdeps/generic/sys/sysmacros.h
deleted file mode 100644
index 4cc5961..0000000
--- a/sysdeps/generic/sys/sysmacros.h
+++ /dev/null
@@ -1,30 +0,0 @@
-/* Definitions of macros to access `dev_t' values.
-   Copyright (C) 1996-2016 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#ifndef _SYS_SYSMACROS_H
-#define _SYS_SYSMACROS_H	1
-
-/* For compatibility we provide alternative names.
-
-   The problem here is that compilers other than GCC probably don't
-   have the `long long' type and so `dev_t' is actually an array.  */
-#define major(dev) ((int)(((unsigned int) (dev) >> 8) & 0xff))
-#define minor(dev) ((int)((dev) & 0xff))
-#define makedev(major, minor) (((major) << 8) | (minor))
-
-#endif /* sys/sysmacros.h */
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 35e1ed4..f0b052d 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -15,7 +15,7 @@ ifeq ($(subdir),misc)
 include $(firstword $(wildcard $(sysdirs:=/sysctl.mk)))
 
 sysdep_routines += clone llseek umount umount2 readahead \
-		   setfsuid setfsgid makedev epoll_pwait signalfd \
+		   setfsuid setfsgid epoll_pwait signalfd \
 		   eventfd eventfd_read eventfd_write prlimit \
 		   personality
 
diff --git a/sysdeps/unix/sysv/linux/makedev.c b/sysdeps/unix/sysv/linux/makedev.c
deleted file mode 100644
index 68c18ca..0000000
--- a/sysdeps/unix/sysv/linux/makedev.c
+++ /dev/null
@@ -1,40 +0,0 @@
-/* Definitions of functions to access `dev_t' values.
-   Copyright (C) 2003-2016 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#include <endian.h>
-#include <sys/sysmacros.h>
-
-unsigned int
-gnu_dev_major (unsigned long long int dev)
-{
-  return ((dev >> 8) & 0xfff) | ((unsigned int) (dev >> 32) & ~0xfff);
-}
-
-unsigned int
-gnu_dev_minor (unsigned long long int dev)
-{
-  return (dev & 0xff) | ((unsigned int) (dev >> 12) & ~0xff);
-}
-
-unsigned long long int
-gnu_dev_makedev (unsigned int major, unsigned int minor)
-{
-  return ((minor & 0xff) | ((major & 0xfff) << 8)
-	  | (((unsigned long long int) (minor & ~0xff)) << 12)
-	  | (((unsigned long long int) (major & ~0xfff)) << 32));
-}
diff --git a/sysdeps/unix/sysv/linux/sys/sysmacros.h b/sysdeps/unix/sysv/linux/sys/sysmacros.h
deleted file mode 100644
index 4c4a697..0000000
--- a/sysdeps/unix/sysv/linux/sys/sysmacros.h
+++ /dev/null
@@ -1,65 +0,0 @@
-/* Definitions of macros to access `dev_t' values.
-   Copyright (C) 1996-2016 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <http://www.gnu.org/licenses/>.  */
-
-#ifndef _SYS_SYSMACROS_H
-#define _SYS_SYSMACROS_H	1
-
-#include <features.h>
-
-__BEGIN_DECLS
-
-__extension__
-extern unsigned int gnu_dev_major (unsigned long long int __dev)
-     __THROW __attribute_const__;
-__extension__
-extern unsigned int gnu_dev_minor (unsigned long long int __dev)
-     __THROW __attribute_const__;
-__extension__
-extern unsigned long long int gnu_dev_makedev (unsigned int __major,
-					       unsigned int __minor)
-     __THROW __attribute_const__;
-
-#ifdef __USE_EXTERN_INLINES
-__extension__ __extern_inline __attribute_const__ unsigned int
-__NTH (gnu_dev_major (unsigned long long int __dev))
-{
-  return ((__dev >> 8) & 0xfff) | ((unsigned int) (__dev >> 32) & ~0xfff);
-}
-
-__extension__ __extern_inline __attribute_const__ unsigned int
-__NTH (gnu_dev_minor (unsigned long long int __dev))
-{
-  return (__dev & 0xff) | ((unsigned int) (__dev >> 12) & ~0xff);
-}
-
-__extension__ __extern_inline __attribute_const__ unsigned long long int
-__NTH (gnu_dev_makedev (unsigned int __major, unsigned int __minor))
-{
-  return ((__minor & 0xff) | ((__major & 0xfff) << 8)
-	  | (((unsigned long long int) (__minor & ~0xff)) << 12)
-	  | (((unsigned long long int) (__major & ~0xfff)) << 32));
-}
-#endif
-__END_DECLS
-
-/* Access the functions with their traditional names.  */
-#define major(dev) gnu_dev_major (dev)
-#define minor(dev) gnu_dev_minor (dev)
-#define makedev(maj, min) gnu_dev_makedev (maj, min)
-
-#endif /* sys/sysmacros.h */
-- 
2.8.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [2.24 PATCH 0/3] sys/sysmacros.h and sys.types.h, revised^n
@ 2016-07-31 20:09 Zack Weinberg
  2016-07-31 20:09 ` [2.24 PATCH 2/3] Minimize sysdeps code involved in defining major/minor/makedev Zack Weinberg
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Zack Weinberg @ 2016-07-31 20:09 UTC (permalink / raw)
  To: libc-alpha; +Cc: adhemerval.zanella, vapier, carlos

I've rebased the patches for deprecating inclusion of
<sys/sysmacros.h> by <sys/types.h> against latest trunk (now with 96%
fewer changes to .abilist files!)  I have also redesigned the new
bits/sysmacros.h mechanism yet again; I hope this will address Mike
Frysinger's stylistic concerns.

It's my understanding that this series is still wanted for 2.24.  I
will need someone to commit it for me (how do I apply for commit
privileges? this is becoming tedious)

zw

Zack Weinberg (3):
  Add utility macros for clang detection, and deprecation with messages.
  Minimize sysdeps code involved in defining major/minor/makedev.
  Deprecate inclusion of <sys/sysmacros.h> by <sys/types.h>

 NEWS                                    |  16 +++++
 bits/sysmacros.h                        |  74 ++++++++++++++++++++
 include/features.h                      |  19 +++--
 include/sys/sysmacros.h                 |   1 +
 misc/Makefile                           |   7 +-
 misc/Versions                           |   3 +
 misc/makedev.c                          |  30 ++++++++
 misc/sys/cdefs.h                        |  22 +++++-
 misc/sys/sysmacros.h                    | 120 ++++++++++++++++++++++++++++++++
 misc/tst-makedev.c                      | 104 +++++++++++++++++++++++++++
 posix/Makefile                          |   2 +-
 posix/sys/types.h                       |   8 ++-
 sysdeps/arm/nacl/libc.abilist           |   4 ++
 sysdeps/generic/sys/sysmacros.h         |  30 --------
 sysdeps/unix/sysv/linux/Makefile        |   2 +-
 sysdeps/unix/sysv/linux/makedev.c       |  40 -----------
 sysdeps/unix/sysv/linux/sys/sysmacros.h |  65 -----------------
 17 files changed, 401 insertions(+), 146 deletions(-)
 create mode 100644 bits/sysmacros.h
 create mode 100644 include/sys/sysmacros.h
 create mode 100644 misc/makedev.c
 create mode 100644 misc/sys/sysmacros.h
 create mode 100644 misc/tst-makedev.c
 delete mode 100644 sysdeps/generic/sys/sysmacros.h
 delete mode 100644 sysdeps/unix/sysv/linux/makedev.c
 delete mode 100644 sysdeps/unix/sysv/linux/sys/sysmacros.h

-- 
2.8.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [2.24 PATCH 3/3] Deprecate inclusion of <sys/sysmacros.h> by <sys/types.h>
  2016-07-31 20:09 [2.24 PATCH 0/3] sys/sysmacros.h and sys.types.h, revised^n Zack Weinberg
  2016-07-31 20:09 ` [2.24 PATCH 2/3] Minimize sysdeps code involved in defining major/minor/makedev Zack Weinberg
@ 2016-07-31 20:09 ` Zack Weinberg
  2016-08-01 21:07   ` Carlos O'Donell
       [not found] ` <096b80f58b0361a9b34b2eb92e370b0592d15971.1469994984.git.zackw@panix.com>
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Zack Weinberg @ 2016-07-31 20:09 UTC (permalink / raw)
  To: libc-alpha; +Cc: adhemerval.zanella, vapier, carlos

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.
---
 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.
+
+  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.
+
 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 "
+
 #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));
+
 #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)
+
 __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)
+
 #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)
+
 #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
 
 #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.  */
+# define __SYSMACROS_DEPRECATED_INCLUSION
 # include <sys/sysmacros.h>
+# undef __SYSMACROS_DEPRECATED_INCLUSION
 #endif /* Use misc.  */
 
 
-- 
2.8.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 1/3] Add utility macros for clang detection, and deprecation with messages.
       [not found] ` <096b80f58b0361a9b34b2eb92e370b0592d15971.1469994984.git.zackw@panix.com>
@ 2016-08-01  3:29   ` Paul Eggert
  2016-08-01 14:20     ` Zack Weinberg
  2016-08-01 20:09   ` Carlos O'Donell
  1 sibling, 1 reply; 20+ messages in thread
From: Paul Eggert @ 2016-08-01  3:29 UTC (permalink / raw)
  To: Zack Weinberg, libc-alpha; +Cc: adhemerval.zanella, vapier, carlos

Zack Weinberg wrote:
> +#if __GNUC_PREREQ (4,5) || \
> +    __glibc_clang_has_extension (__attribute_deprecated_with_message__)

This sort of condition should be reworded to be simpler, like this:

#if __glibc_has_extension (__attribute_deprecated_with_message__)

with an implementation that looks something like this:

#ifdef __has_extension
# define __glibc_has_extension(ext) __has_extension (ext)
#else
# define __glibc_has_extension(ext) __glibc_has_extension##ext
# define __glibc_has_extension__attribute_deprecated_with_message__ 
__GNUC_PREREQ (4,5)
#endif

This will be easier to maintain, as only the implementation will need to worry 
about GCC version numbers.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 1/3] Add utility macros for clang detection, and deprecation with messages.
  2016-08-01  3:29   ` [2.24 PATCH 1/3] Add utility macros for clang detection, and deprecation with messages Paul Eggert
@ 2016-08-01 14:20     ` Zack Weinberg
  2016-08-01 21:36       ` Paul Eggert
  0 siblings, 1 reply; 20+ messages in thread
From: Zack Weinberg @ 2016-08-01 14:20 UTC (permalink / raw)
  To: Paul Eggert
  Cc: GNU C Library, Adhemerval Zanella, Mike Frysinger, Carlos O'Donell

On Sun, Jul 31, 2016 at 11:29 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> Zack Weinberg wrote:
>>
>> +#if __GNUC_PREREQ (4,5) || \
>> +    __glibc_clang_has_extension (__attribute_deprecated_with_message__)
>
> This sort of condition should be reworded to be simpler, like this:
[...]

Roland asked for something similar when we were discussing an earlier
revision.  I've thought about it a bunch and I have concluded that it
does not actually make _this particular situation_ any simpler.  Right
now, there's only one conditional, controlling how
__attribute_deprecated_with_message__ is defined.  If I were to
introduce the mechanism you want, we would have _two_ conditionals
involved in the definition of that macro.

It might still be a good idea as part of a complete revision of
sys/cdefs.h + features.h, subsuming _all_ of the GCC version checks
into this mechanism and rendering it easily extensible to more
different compilers -- which is what Roland wanted -- but that is a
separate project, not appropriate for 2.24 at this stage, and I do not
want to mission-creep _this_ patchset even to the extent of
introducing the skeleton of the mechanism.

zw

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 0/3] sys/sysmacros.h and sys.types.h, revised^n
  2016-07-31 20:09 [2.24 PATCH 0/3] sys/sysmacros.h and sys.types.h, revised^n Zack Weinberg
                   ` (2 preceding siblings ...)
       [not found] ` <096b80f58b0361a9b34b2eb92e370b0592d15971.1469994984.git.zackw@panix.com>
@ 2016-08-01 18:47 ` Carlos O'Donell
  2016-08-01 20:38 ` Carlos O'Donell
  4 siblings, 0 replies; 20+ messages in thread
From: Carlos O'Donell @ 2016-08-01 18:47 UTC (permalink / raw)
  To: Zack Weinberg, libc-alpha; +Cc: adhemerval.zanella, vapier

On 07/31/2016 04:09 PM, Zack Weinberg wrote:
> (how do I apply for commit
> privileges? this is becoming tedious)

https://sourceware.org/glibc/wiki/MAINTAINERS#Source_Control_ACLs

There is an entire series of Zack-isms still in use at CodeSourcery
and I find myself saying one of them at least once a year,
particularly "belt-and-suspenders."

I will vouch for your sanity and competence :-)

-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 1/3] Add utility macros for clang detection, and deprecation with messages.
       [not found] ` <096b80f58b0361a9b34b2eb92e370b0592d15971.1469994984.git.zackw@panix.com>
  2016-08-01  3:29   ` [2.24 PATCH 1/3] Add utility macros for clang detection, and deprecation with messages Paul Eggert
@ 2016-08-01 20:09   ` Carlos O'Donell
  2016-08-01 20:19     ` Zack Weinberg
  1 sibling, 1 reply; 20+ messages in thread
From: Carlos O'Donell @ 2016-08-01 20:09 UTC (permalink / raw)
  To: Zack Weinberg, libc-alpha; +Cc: adhemerval.zanella, vapier

On 07/31/2016 04:09 PM, Zack Weinberg wrote:
> There are three new macros added to features.h and sys/cdefs.h:
> 
>  * __glibc_clang_prereq: just like __GNUC_PREREQ, but for clang.
>  * __glibc_clang_has_extension: wraps clang's intrinsic __has_extension.
>    Writing "#if defined __clang__ && __has_extension (...)" doesn't work,
>    because compilers other than clang will object to the unknown macro
>    __has_extension even though they don't need to evaluate it.
>    Instead, write "#if __glibc_clang_has_extension (...)".
> 
>  * __attribute_deprecated_msg__(msg): like __attribute_deprecated__, but
>    if possible, prints a message.
> 
> The first two are used to define the third.  The third will be used
> in subsequent patches.
> 
> ChangeLog:
> 	* include/features.h (__glibc_clang_prereq): New macro.
> 	* misc/sys/cdefs.h (__glibc_clang_has_extension)
> 	(__attribute_deprecated_msg__): New macros.

This looks good to me. I have no objections like Roland or Paul about
abstracting this into some generic infrastructure. These things should
grow organically based on need IMO.

-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 1/3] Add utility macros for clang detection, and deprecation with messages.
  2016-08-01 20:09   ` Carlos O'Donell
@ 2016-08-01 20:19     ` Zack Weinberg
  2016-08-01 20:46       ` Carlos O'Donell
  0 siblings, 1 reply; 20+ messages in thread
From: Zack Weinberg @ 2016-08-01 20:19 UTC (permalink / raw)
  To: Carlos O'Donell; +Cc: GNU C Library, Adhemerval Zanella, Mike Frysinger

On Mon, Aug 1, 2016 at 4:09 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>
> This looks good to me. I have no objections like Roland or Paul about
> abstracting this into some generic infrastructure. These things should
> grow organically based on need IMO.

For the record, I do have the generic infrastructure on my to-do list
for 2.25, but I cannot promise to actually get to it.

zw

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 0/3] sys/sysmacros.h and sys.types.h, revised^n
  2016-07-31 20:09 [2.24 PATCH 0/3] sys/sysmacros.h and sys.types.h, revised^n Zack Weinberg
                   ` (3 preceding siblings ...)
  2016-08-01 18:47 ` [2.24 PATCH 0/3] sys/sysmacros.h and sys.types.h, revised^n Carlos O'Donell
@ 2016-08-01 20:38 ` Carlos O'Donell
  2016-08-01 20:50   ` Adhemerval Zanella
  4 siblings, 1 reply; 20+ messages in thread
From: Carlos O'Donell @ 2016-08-01 20:38 UTC (permalink / raw)
  To: Zack Weinberg, libc-alpha, adhemerval.zanella; +Cc: vapier

On 07/31/2016 04:09 PM, Zack Weinberg wrote:
> I've rebased the patches for deprecating inclusion of
> <sys/sysmacros.h> by <sys/types.h> against latest trunk (now with 96%
> fewer changes to .abilist files!)  I have also redesigned the new
> bits/sysmacros.h mechanism yet again; I hope this will address Mike
> Frysinger's stylistic concerns.
> 
> It's my understanding that this series is still wanted for 2.24.  I
> will need someone to commit it for me (how do I apply for commit
> privileges? this is becoming tedious)
> 
> zw
> 
> Zack Weinberg (3):
>   Add utility macros for clang detection, and deprecation with messages.
>   Minimize sysdeps code involved in defining major/minor/makedev.
>   Deprecate inclusion of <sys/sysmacros.h> by <sys/types.h>

Zack,

These changes should have gone in earlier, but you don't have commit
access, and review for 2.24 for myself has been tied up because of other
projects I had to handle. With 2.25 I hope you have commit access and get
these kinds of changes in earlier, they have lots of value and I enjoy
seeing responsible people fixing things.

So here is the problem:

* Today is the branch cut day for 2.24.

* This patch changes at least one libc.abilist file.

* This patch changes at least one Versions file.

Despite my strong desire to get this into 2.24 I find that it's simply
too late. I'm sorry.

Therefore I'm faced with the conclusion that I'd like to cut the branch
and _then_ commit these patches for you right away.

Adhemerval,

Thoughts?

-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 1/3] Add utility macros for clang detection, and deprecation with messages.
  2016-08-01 20:19     ` Zack Weinberg
@ 2016-08-01 20:46       ` Carlos O'Donell
  0 siblings, 0 replies; 20+ messages in thread
From: Carlos O'Donell @ 2016-08-01 20:46 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: GNU C Library, Adhemerval Zanella, Mike Frysinger

On 08/01/2016 04:19 PM, Zack Weinberg wrote:
> On Mon, Aug 1, 2016 at 4:09 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>>
>> This looks good to me. I have no objections like Roland or Paul about
>> abstracting this into some generic infrastructure. These things should
>> grow organically based on need IMO.
> 
> For the record, I do have the generic infrastructure on my to-do list
> for 2.25, but I cannot promise to actually get to it.

I appreciate any of the valuable volunteer time you give to the project.

-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 0/3] sys/sysmacros.h and sys.types.h, revised^n
  2016-08-01 20:38 ` Carlos O'Donell
@ 2016-08-01 20:50   ` Adhemerval Zanella
  2016-08-01 21:25     ` Zack Weinberg
  0 siblings, 1 reply; 20+ messages in thread
From: Adhemerval Zanella @ 2016-08-01 20:50 UTC (permalink / raw)
  To: Carlos O'Donell, Zack Weinberg, libc-alpha; +Cc: vapier



On 01/08/2016 17:38, Carlos O'Donell wrote:
> On 07/31/2016 04:09 PM, Zack Weinberg wrote:
>> I've rebased the patches for deprecating inclusion of
>> <sys/sysmacros.h> by <sys/types.h> against latest trunk (now with 96%
>> fewer changes to .abilist files!)  I have also redesigned the new
>> bits/sysmacros.h mechanism yet again; I hope this will address Mike
>> Frysinger's stylistic concerns.
>>
>> It's my understanding that this series is still wanted for 2.24.  I
>> will need someone to commit it for me (how do I apply for commit
>> privileges? this is becoming tedious)
>>
>> zw
>>
>> Zack Weinberg (3):
>>   Add utility macros for clang detection, and deprecation with messages.
>>   Minimize sysdeps code involved in defining major/minor/makedev.
>>   Deprecate inclusion of <sys/sysmacros.h> by <sys/types.h>
> 
> Zack,
> 
> These changes should have gone in earlier, but you don't have commit
> access, and review for 2.24 for myself has been tied up because of other
> projects I had to handle. With 2.25 I hope you have commit access and get
> these kinds of changes in earlier, they have lots of value and I enjoy
> seeing responsible people fixing things.
> 
> So here is the problem:
> 
> * Today is the branch cut day for 2.24.
> 
> * This patch changes at least one libc.abilist file.
> 
> * This patch changes at least one Versions file.
> 
> Despite my strong desire to get this into 2.24 I find that it's simply
> too late. I'm sorry.
> 
> Therefore I'm faced with the conclusion that I'd like to cut the branch
> and _then_ commit these patches for you right away.
> 
> Adhemerval,
> 
> Thoughts?
> 

I feel the same and it is just a timeframe issue.  Thanks for checking on
this Carlos.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 2/3] Minimize sysdeps code involved in defining major/minor/makedev.
  2016-07-31 20:09 ` [2.24 PATCH 2/3] Minimize sysdeps code involved in defining major/minor/makedev Zack Weinberg
@ 2016-08-01 20:55   ` Carlos O'Donell
  2016-08-01 21:28     ` Zack Weinberg
  0 siblings, 1 reply; 20+ messages in thread
From: Carlos O'Donell @ 2016-08-01 20:55 UTC (permalink / raw)
  To: Zack Weinberg, libc-alpha; +Cc: adhemerval.zanella, vapier

On 07/31/2016 04:09 PM, Zack Weinberg wrote:
> Presently sys/sysmacros.h is entirely defined in sysdeps.  This would
> mean that the deprecation logic coming up in the next patch would have
> to be written twice (in generic/ and unix/sysv/linux/).  To avoid that,
> hoist all but the unavoidably system-dependent logic to misc/, leaving a
> bits/ header behind.  This also promotes the Linux-specific encoding of
> dev_t, which accommodates 32-bit major and minor numbers in a 64-bit dev_t,
> to generic, as glibc's dev_t is always 64 bits wide.
> 
> The former Linux implementation used inline functions to avoid evaluating
> arguments more than once.  After this change, all platforms use inline
> functions, which means that three new symbols are added to the generic ABI.
> Since NaCl is the only non-Linux port currently in the tree, the only
> libc.abilist file that requires a change is arm/nacl/libc.abilist.
> 
> New ports henceforth need only provide a bits/sysmacros.h defining
> internal macros __SYSMACROS_{DECLARE,DEFINE}_{MAJOR,MINOR,MAKEDEV}.
> 
> While I was at it, I added a basic round-trip test for these functions.

(1) Inclusion in 2.25?

I have already made comments that I think we need to put this into 2.25
at this late stage. If you can answer (3) I can commit this immeidately
once 2.25 opens.

(2) Testing?

Awesome test.

(3) New symbols?

Overall this looks good. I have only questions about why the new functions
are going into the users namespace. Traditionally we put things into the
implementation namespace, particularly to discourage users calling the
functions directly. I would like the user to use the normal interfaces for
calling this functionality.

> ChangeLog:
> 
> 	* sysdeps/generic/sys/sysmacros.h: Delete file.
> 	* sysdeps/unix/sysv/linux/makedev.c: Delete file.
> 	* sysdeps/unix/sysv/linux/sys/sysmacros.h: Move file ...
> 	* bits/sysmacros.h: ... here; this encoding is now the generic
> 	encoding.  Now defines only the following macros:
> 	__SYSMACROS_DECLARE_MAJOR, __SYSMACROS_DEFINE_MAJOR,
> 	__SYSMACROS_DECLARE_MINOR, __SYSMACROS_DEFINE_MINOR,
> 	__SYSMACROS_DECLARE_MAKEDEV, __SYSMACROS_DEFINE_MAKEDEV.
> 
> 	* misc/sys/sysmacros.h, misc/makedev.c: New files that use
> 	bits/sysmacros.h and the above new macros to generate the
> 	public implementations of major, minor, and makedev.
> 	* misc/tst-makedev.c: New test.
> 	* include/sys/sysmacros.h: New wrapper.
> 
> 	* misc/Makefile (headers): Add sys/sysmacros.h, bits/sysmacros.h.
> 	(routines): Add makedev.
> 	(tests): Add tst-makedev.
> 	* misc/Versions [GLIBC_2.24]: Add gnu_dev_major, gnu_dev_minor,
> 	gnu_dev_makedev.
> 	* posix/Makefile (headers): Remove sys/sysmacros.h.
> 	* sysdeps/unix/sysv/linux/Makefile (sysdep_routines): Remove makedev.
> 
> 	* sysdeps/arm/nacl/libc.abilist: Add GLIBC_2.24,
> 	gnu_dev_major, gnu_dev_makedev, gnu_dev_minor.
> ---
>  bits/sysmacros.h                        |  74 +++++++++++++++++++++++
>  include/sys/sysmacros.h                 |   1 +
>  misc/Makefile                           |   7 ++-
>  misc/Versions                           |   3 +
>  misc/makedev.c                          |  30 +++++++++
>  misc/sys/sysmacros.h                    |  64 ++++++++++++++++++++
>  misc/tst-makedev.c                      | 104 ++++++++++++++++++++++++++++++++
>  posix/Makefile                          |   2 +-
>  sysdeps/arm/nacl/libc.abilist           |   4 ++
>  sysdeps/generic/sys/sysmacros.h         |  30 ---------
>  sysdeps/unix/sysv/linux/Makefile        |   2 +-
>  sysdeps/unix/sysv/linux/makedev.c       |  40 ------------
>  sysdeps/unix/sysv/linux/sys/sysmacros.h |  65 --------------------
>  13 files changed, 286 insertions(+), 140 deletions(-)
>  create mode 100644 bits/sysmacros.h
>  create mode 100644 include/sys/sysmacros.h
>  create mode 100644 misc/makedev.c
>  create mode 100644 misc/sys/sysmacros.h
>  create mode 100644 misc/tst-makedev.c
>  delete mode 100644 sysdeps/generic/sys/sysmacros.h
>  delete mode 100644 sysdeps/unix/sysv/linux/makedev.c
>  delete mode 100644 sysdeps/unix/sysv/linux/sys/sysmacros.h
> 
> diff --git a/bits/sysmacros.h b/bits/sysmacros.h
> new file mode 100644
> index 0000000..e2c0bc2
> --- /dev/null
> +++ b/bits/sysmacros.h
> @@ -0,0 +1,74 @@
> +/* Definitions of macros to access `dev_t' values.
> +   Copyright (C) 1996-2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _BITS_SYSMACROS_H
> +#define _BITS_SYSMACROS_H 1
> +
> +#ifndef _SYS_SYSMACROS_H
> +# error "Never include <bits/sysmacros.h> directly; use <sys/sysmacros.h> instead."
> +#endif
> +
> +/* dev_t in glibc is a 64-bit quantity, with 32-bit major and minor numbers.
> +   Our default encoding is MMMM Mmmm mmmM MMmm, where M is a hex digit of
> +   the major number and m is a hex digit of the minor number.  This is
> +   downward compatible with legacy systems where dev_t is 16 bits wide,
> +   encoded as MMmm.  It is also downward compatible with the Linux kernel,
> +   which (as of 2016) uses 32-bit dev_t, encoded as mmmM MMmm.
> +
> +   Systems that use an incompatible encoding for dev_t should override this
> +   file in the appropriate sysdeps subdirectory.  */
> +
> +#define __SYSMACROS_DECLARE_MAJOR(DECL_TEMPL)			\
> +  DECL_TEMPL(unsigned int, major, (__dev_t __dev))
> +
> +#define __SYSMACROS_DEFINE_MAJOR(DECL_TEMPL)			\
> +  __SYSMACROS_DECLARE_MAJOR (DECL_TEMPL)			\
> +  {								\
> +    unsigned int __major;					\
> +    __major  = ((__dev & (__dev_t) 0x00000000000fff00u) >>  8); \
> +    __major |= ((__dev & (__dev_t) 0xfffff00000000000u) >> 32); \

OK.

> +    return __major;						\
> +  }
> +
> +#define __SYSMACROS_DECLARE_MINOR(DECL_TEMPL)			\
> +  DECL_TEMPL(unsigned int, minor, (__dev_t __dev))
> +
> +#define __SYSMACROS_DEFINE_MINOR(DECL_TEMPL)			\
> +  __SYSMACROS_DECLARE_MINOR (DECL_TEMPL)			\
> +  {								\
> +    unsigned int __minor;					\
> +    __minor  = ((__dev & (__dev_t) 0x00000000000000ffu) >>  0); \
> +    __minor |= ((__dev & (__dev_t) 0x00000ffffff00000u) >> 12); \

OK.

> +    return __minor;						\
> +  }
> +
> +#define __SYSMACROS_DECLARE_MAKEDEV(DECL_TEMPL)			\
> +  DECL_TEMPL(__dev_t, makedev, (unsigned int __major, unsigned int __minor))
> +
> +#define __SYSMACROS_DEFINE_MAKEDEV(DECL_TEMPL)			\
> +  __SYSMACROS_DECLARE_MAKEDEV (DECL_TEMPL)			\
> +  {								\
> +    __dev_t __dev;						\
> +    __dev  = (((__dev_t) (__major & 0x00000fffu)) <<  8);	\
> +    __dev |= (((__dev_t) (__major & 0xfffff000u)) << 32);	\
> +    __dev |= (((__dev_t) (__minor & 0x000000ffu)) <<  0);	\
> +    __dev |= (((__dev_t) (__minor & 0xffffff00u)) << 12);	\

OK.

> -  return ((minor & 0xff) | ((major & 0xfff) << 8)
> -	  | (((unsigned long long int) (minor & ~0xff)) << 12)
> -	  | (((unsigned long long int) (major & ~0xfff)) << 32));

OK. Verified matches the old code.

> +    return __dev;						\
> +  }
> +
> +#endif /* bits/sysmacros.h */
> diff --git a/include/sys/sysmacros.h b/include/sys/sysmacros.h
> new file mode 100644
> index 0000000..87813c5
> --- /dev/null
> +++ b/include/sys/sysmacros.h
> @@ -0,0 +1 @@
> +#include <misc/sys/sysmacros.h>
> diff --git a/misc/Makefile b/misc/Makefile
> index 56e20ce..3d2ebb8 100644
> --- a/misc/Makefile
> +++ b/misc/Makefile
> @@ -34,7 +34,8 @@ headers	:= sys/uio.h bits/uio.h sys/ioctl.h bits/ioctls.h bits/ioctl-types.h \
>  	   regexp.h bits/select.h bits/mman.h sys/xattr.h \
>  	   syslog.h sys/syslog.h \
>  	   bits/syslog.h bits/syslog-ldbl.h bits/syslog-path.h bits/error.h \
> -	   bits/select2.h bits/hwcap.h sys/auxv.h
> +	   bits/select2.h bits/hwcap.h sys/auxv.h \
> +	   sys/sysmacros.h bits/sysmacros.h

OK.

>  
>  routines := brk sbrk sstk ioctl \
>  	    readv writev preadv preadv64 pwritev pwritev64 \
> @@ -67,7 +68,7 @@ routines := brk sbrk sstk ioctl \
>  	    getloadavg getclktck \
>  	    fgetxattr flistxattr fremovexattr fsetxattr getxattr \
>  	    listxattr lgetxattr llistxattr lremovexattr lsetxattr \
> -	    removexattr setxattr getauxval ifunc-impl-list
> +	    removexattr setxattr getauxval ifunc-impl-list makedev

OK.

>  
>  generated += tst-error1.mtrace tst-error1-mem.out
>  
> @@ -78,7 +79,7 @@ gpl2lgpl := error.c error.h
>  tests := tst-dirname tst-tsearch tst-fdset tst-efgcvt tst-mntent tst-hsearch \
>  	 tst-error1 tst-pselect tst-insremque tst-mntent2 bug-hsearch1 \
>  	 tst-mntent-blank-corrupt tst-mntent-blank-passno bug18240 \
> -	 tst-preadvwritev tst-preadvwritev64
> +	 tst-preadvwritev tst-preadvwritev64 tst-makedev

OK.

>  ifeq ($(run-built-tests),yes)
>  tests-special += $(objpfx)tst-error1-mem.out
>  endif
> diff --git a/misc/Versions b/misc/Versions
> index 671f487..afa22bd 100644
> --- a/misc/Versions
> +++ b/misc/Versions
> @@ -152,6 +152,9 @@ libc {
>    GLIBC_2.23 {
>      # SHLIB_COMPAT(GLIBC_2_0, GLIBC_2_23) used in regexp.c
>    }
> +  GLIBC_2.24 {
> +    gnu_dev_major; gnu_dev_minor; gnu_dev_makedev;
> +  }

Not OK.

Traditionally I thought we hid internal functions like this in the
implemetnation namespace to avoid the suggestion that users could call
them directly. In this case they can, but still, it's a user namespace.

Why do we put these in the user namespace?


>    GLIBC_PRIVATE {
>      __madvise;
>      __mktemp;
> diff --git a/misc/makedev.c b/misc/makedev.c
> new file mode 100644
> index 0000000..7303fb6
> --- /dev/null
> +++ b/misc/makedev.c
> @@ -0,0 +1,30 @@
> +/* Definitions of functions to access `dev_t' values.
> +   Copyright (C) 2003-2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <features.h>
> +
> +#undef __USE_EXTERN_INLINES
> +#define __SYSMACROS_NEED_IMPLEMENTATION
> +#include <sys/sysmacros.h>
> +
> +#define OUT_OF_LINE_IMPL_TEMPL(rtype, name, proto) \
> +  rtype gnu_dev_##name proto
> +
> +__SYSMACROS_DEFINE_MAJOR(OUT_OF_LINE_IMPL_TEMPL)
> +__SYSMACROS_DEFINE_MINOR(OUT_OF_LINE_IMPL_TEMPL)
> +__SYSMACROS_DEFINE_MAKEDEV(OUT_OF_LINE_IMPL_TEMPL)

OK.

> diff --git a/misc/sys/sysmacros.h b/misc/sys/sysmacros.h
> new file mode 100644
> index 0000000..dc2eb83
> --- /dev/null
> +++ b/misc/sys/sysmacros.h
> @@ -0,0 +1,64 @@
> +/* Definitions of macros to access `dev_t' values.
> +   Copyright (C) 1996-2015 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef _SYS_SYSMACROS_H
> +#define _SYS_SYSMACROS_H 1
> +
> +#include <features.h>
> +#include <bits/types.h>
> +#include <bits/sysmacros.h>
> +
> +#define __SYSMACROS_DECL_TEMPL(rtype, name, proto)			     \
> +  extern rtype gnu_dev_##name proto __THROW __attribute_const__;
> +
> +#define __SYSMACROS_IMPL_TEMPL(rtype, name, proto)			     \
> +  __extension__ __extern_inline __attribute_const__ rtype		     \
> +  __NTH (gnu_dev_##name proto)
> +
> +__BEGIN_DECLS
> +
> +__SYSMACROS_DECLARE_MAJOR (__SYSMACROS_DECL_TEMPL)
> +__SYSMACROS_DECLARE_MINOR (__SYSMACROS_DECL_TEMPL)
> +__SYSMACROS_DECLARE_MAKEDEV (__SYSMACROS_DECL_TEMPL)
> +
> +#ifdef __USE_EXTERN_INLINES
> +
> +__SYSMACROS_DEFINE_MAJOR (__SYSMACROS_IMPL_TEMPL)
> +__SYSMACROS_DEFINE_MINOR (__SYSMACROS_IMPL_TEMPL)
> +__SYSMACROS_DEFINE_MAKEDEV (__SYSMACROS_IMPL_TEMPL)
> +
> +#endif
> +
> +__END_DECLS
> +
> +#ifndef __SYSMACROS_NEED_IMPLEMENTATION
> +# undef __SYSMACROS_DECL_TEMPL
> +# undef __SYSMACROS_IMPL_TEMPL
> +# undef __SYSMACROS_DECLARE_MAJOR
> +# undef __SYSMACROS_DECLARE_MINOR
> +# undef __SYSMACROS_DECLARE_MAKEDEV
> +# undef __SYSMACROS_DEFINE_MAJOR
> +# undef __SYSMACROS_DEFINE_MINOR
> +# 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)
> +
> +#endif /* sys/sysmacros.h */

OK.

> diff --git a/misc/tst-makedev.c b/misc/tst-makedev.c
> new file mode 100644
> index 0000000..b330b5a
> --- /dev/null
> +++ b/misc/tst-makedev.c
> @@ -0,0 +1,104 @@
> +/* Tests of functions to access `dev_t' values.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <sys/types.h>
> +#include <sys/sysmacros.h>
> +#include <stdio.h>
> +#include <inttypes.h>
> +
> +/* Confirm that makedev (major (d), minor (d)) == d.  */
> +static int
> +do_test_split_combine (dev_t d1)
> +{
> +  unsigned int maj = major (d1);
> +  unsigned int min = minor (d1);
> +  dev_t d2 = makedev (maj, min);
> +  if (d1 != d2)
> +    {
> +      printf ("FAIL: %016" PRIx64 " != %016" PRIx64 " (maj %08x min %08x)\n",
> +	      (uint64_t)d2, (uint64_t)d1, maj, min);
> +      return 1;
> +    }
> +  else
> +    return 0;
> +}
> +
> +/* Confirm that major (makedev (maj, min)) == maj and
> +   minor (makedev (maj, min)) == min.  */
> +static int
> +do_test_combine_split (unsigned int maj1, unsigned int min1)
> +{
> +  dev_t d = makedev (maj1, min1);
> +  unsigned int maj2 = major (d);
> +  unsigned int min2 = minor (d);
> +  if (maj1 != maj2 && min1 != min2)
> +    {
> +      printf ("FAIL: %08x != %08x, %08x != %08x (dev %016" PRIx64 ")\n",
> +	      maj2, maj1, min2, min1, (uint64_t)d);
> +      return 1;
> +    }
> +  else if (maj1 != maj2)
> +    {
> +      printf ("FAIL: %08x != %08x, %08x == %08x (dev %016" PRIx64 ")\n",
> +	      maj2, maj1, min2, min1, (uint64_t)d);
> +      return 1;
> +    }
> +  else if (min1 != min2)
> +    {
> +      printf ("FAIL: %08x == %08x, %08x != %08x (dev %016" PRIx64 ")\n",
> +	      maj2, maj1, min2, min1, (uint64_t)d);
> +      return 1;
> +    }
> +  else
> +    return 0;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  dev_t d;
> +  unsigned int maj, min;
> +  int status = 0;
> +
> +  /* Test the traditional range (16-bit dev_t, 8-bit each maj/min)
> +     exhaustively.  */
> +  for (d = 0; d <= 0xFFFF; d++)
> +    status |= do_test_split_combine (d);

Awesome. Belt-and-suspenders :-)

> +
> +  for (maj = 0; maj <= 0xFF; maj++)
> +    for (min = 0; min <= 0xFF; min++)
> +      status |= do_test_combine_split (maj, min);
> +
> +  /* Test glibc's expanded range (64-bit dev_t, 32-bit each maj/min).
> +     Exhaustive testing would take much too long, instead we shift a
> +     pair of 1-bits over each range.  */

Awesome compromise.

> +  {
> +    unsigned int a, b;
> +    for (a = 0; a <= 63; a++)
> +      do_test_split_combine (((dev_t) 0x03) << a);
> +
> +    for (a = 0; a < 31; a++)
> +      for (b = 0; b <= 31; b++)
> +	do_test_combine_split (0x03u << a, 0x03u << b);
> +  }
> +
> +  return status;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> diff --git a/posix/Makefile b/posix/Makefile
> index 5b0e298..3a7719e 100644
> --- a/posix/Makefile
> +++ b/posix/Makefile
> @@ -29,7 +29,7 @@ headers	:= sys/utsname.h sys/times.h sys/wait.h sys/types.h unistd.h	      \
>  	   bits/local_lim.h tar.h bits/utsname.h bits/confname.h	      \
>  	   bits/waitflags.h bits/waitstatus.h sys/unistd.h sched.h	      \
>  	   bits/sched.h re_comp.h wait.h bits/environments.h cpio.h	      \
> -	   sys/sysmacros.h spawn.h bits/unistd.h
> +	   spawn.h bits/unistd.h

OK.

>  
>  routines :=								      \
>  	uname								      \
> diff --git a/sysdeps/arm/nacl/libc.abilist b/sysdeps/arm/nacl/libc.abilist
> index 2f7751d..87b7139 100644
> --- a/sysdeps/arm/nacl/libc.abilist
> +++ b/sysdeps/arm/nacl/libc.abilist
> @@ -1840,4 +1840,8 @@ GLIBC_2.23 fts64_close F
>  GLIBC_2.23 fts64_open F
>  GLIBC_2.23 fts64_read F
>  GLIBC_2.23 fts64_set F
> +GLIBC_2.24 GLIBC_2.24 A
>  GLIBC_2.24 quick_exit F
> +GLIBC_2.24 gnu_dev_major F
> +GLIBC_2.24 gnu_dev_makedev F
> +GLIBC_2.24 gnu_dev_minor F

As discussed about, why not __gnu_dev_major etc. ?

> diff --git a/sysdeps/generic/sys/sysmacros.h b/sysdeps/generic/sys/sysmacros.h
> deleted file mode 100644
> index 4cc5961..0000000
> --- a/sysdeps/generic/sys/sysmacros.h
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -/* Definitions of macros to access `dev_t' values.
> -   Copyright (C) 1996-2016 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#ifndef _SYS_SYSMACROS_H
> -#define _SYS_SYSMACROS_H	1
> -
> -/* For compatibility we provide alternative names.
> -
> -   The problem here is that compilers other than GCC probably don't
> -   have the `long long' type and so `dev_t' is actually an array.  */
> -#define major(dev) ((int)(((unsigned int) (dev) >> 8) & 0xff))
> -#define minor(dev) ((int)((dev) & 0xff))
> -#define makedev(major, minor) (((major) << 8) | (minor))
> -
> -#endif /* sys/sysmacros.h */

OK.

> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 35e1ed4..f0b052d 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -15,7 +15,7 @@ ifeq ($(subdir),misc)
>  include $(firstword $(wildcard $(sysdirs:=/sysctl.mk)))
>  
>  sysdep_routines += clone llseek umount umount2 readahead \
> -		   setfsuid setfsgid makedev epoll_pwait signalfd \
> +		   setfsuid setfsgid epoll_pwait signalfd \

OK.

>  		   eventfd eventfd_read eventfd_write prlimit \
>  		   personality
>  
> diff --git a/sysdeps/unix/sysv/linux/makedev.c b/sysdeps/unix/sysv/linux/makedev.c
> deleted file mode 100644
> index 68c18ca..0000000
> --- a/sysdeps/unix/sysv/linux/makedev.c
> +++ /dev/null
> @@ -1,40 +0,0 @@
> -/* Definitions of functions to access `dev_t' values.
> -   Copyright (C) 2003-2016 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#include <endian.h>
> -#include <sys/sysmacros.h>
> -
> -unsigned int
> -gnu_dev_major (unsigned long long int dev)
> -{
> -  return ((dev >> 8) & 0xfff) | ((unsigned int) (dev >> 32) & ~0xfff);
> -}
> -
> -unsigned int
> -gnu_dev_minor (unsigned long long int dev)
> -{
> -  return (dev & 0xff) | ((unsigned int) (dev >> 12) & ~0xff);
> -}
> -
> -unsigned long long int
> -gnu_dev_makedev (unsigned int major, unsigned int minor)
> -{
> -  return ((minor & 0xff) | ((major & 0xfff) << 8)
> -	  | (((unsigned long long int) (minor & ~0xff)) << 12)
> -	  | (((unsigned long long int) (major & ~0xfff)) << 32));
> -}

OK.

> diff --git a/sysdeps/unix/sysv/linux/sys/sysmacros.h b/sysdeps/unix/sysv/linux/sys/sysmacros.h
> deleted file mode 100644
> index 4c4a697..0000000
> --- a/sysdeps/unix/sysv/linux/sys/sysmacros.h
> +++ /dev/null
> @@ -1,65 +0,0 @@
> -/* Definitions of macros to access `dev_t' values.
> -   Copyright (C) 1996-2016 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <http://www.gnu.org/licenses/>.  */
> -
> -#ifndef _SYS_SYSMACROS_H
> -#define _SYS_SYSMACROS_H	1
> -
> -#include <features.h>
> -
> -__BEGIN_DECLS
> -
> -__extension__
> -extern unsigned int gnu_dev_major (unsigned long long int __dev)
> -     __THROW __attribute_const__;
> -__extension__
> -extern unsigned int gnu_dev_minor (unsigned long long int __dev)
> -     __THROW __attribute_const__;
> -__extension__
> -extern unsigned long long int gnu_dev_makedev (unsigned int __major,
> -					       unsigned int __minor)
> -     __THROW __attribute_const__;
> -
> -#ifdef __USE_EXTERN_INLINES
> -__extension__ __extern_inline __attribute_const__ unsigned int
> -__NTH (gnu_dev_major (unsigned long long int __dev))
> -{
> -  return ((__dev >> 8) & 0xfff) | ((unsigned int) (__dev >> 32) & ~0xfff);
> -}
> -
> -__extension__ __extern_inline __attribute_const__ unsigned int
> -__NTH (gnu_dev_minor (unsigned long long int __dev))
> -{
> -  return (__dev & 0xff) | ((unsigned int) (__dev >> 12) & ~0xff);
> -}
> -
> -__extension__ __extern_inline __attribute_const__ unsigned long long int
> -__NTH (gnu_dev_makedev (unsigned int __major, unsigned int __minor))
> -{
> -  return ((__minor & 0xff) | ((__major & 0xfff) << 8)
> -	  | (((unsigned long long int) (__minor & ~0xff)) << 12)
> -	  | (((unsigned long long int) (__major & ~0xfff)) << 32));
> -}
> -#endif
> -__END_DECLS
> -
> -/* Access the functions with their traditional names.  */
> -#define major(dev) gnu_dev_major (dev)
> -#define minor(dev) gnu_dev_minor (dev)
> -#define makedev(maj, min) gnu_dev_makedev (maj, min)
> -
> -#endif /* sys/sysmacros.h */
 
OK.

-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 3/3] Deprecate inclusion of <sys/sysmacros.h> by <sys/types.h>
  2016-07-31 20:09 ` [2.24 PATCH 3/3] Deprecate inclusion of <sys/sysmacros.h> by <sys/types.h> Zack Weinberg
@ 2016-08-01 21:07   ` Carlos O'Donell
  2016-08-03 18:52     ` Zack Weinberg
       [not found]     ` <71ede440-4ace-161f-1848-4475fba05d0f@panix.com>
  0 siblings, 2 replies; 20+ messages in thread
From: Carlos O'Donell @ 2016-08-01 21:07 UTC (permalink / raw)
  To: Zack Weinberg, libc-alpha; +Cc: adhemerval.zanella, vapier

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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 0/3] sys/sysmacros.h and sys.types.h, revised^n
  2016-08-01 20:50   ` Adhemerval Zanella
@ 2016-08-01 21:25     ` Zack Weinberg
  0 siblings, 0 replies; 20+ messages in thread
From: Zack Weinberg @ 2016-08-01 21:25 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: Carlos O'Donell, GNU C Library, Mike Frysinger

On Mon, Aug 1, 2016 at 4:50 PM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
> On 01/08/2016 17:38, Carlos O'Donell wrote:
>> On 07/31/2016 04:09 PM, Zack Weinberg wrote:
>>> I've rebased the patches for deprecating inclusion of
>>> <sys/sysmacros.h> by <sys/types.h> against latest trunk (now with 96%
>>> fewer changes to .abilist files!)  I have also redesigned the new
>>> bits/sysmacros.h mechanism yet again; I hope this will address Mike
>>> Frysinger's stylistic concerns.
>>>
>>> It's my understanding that this series is still wanted for 2.24.  I
>>> will need someone to commit it for me (how do I apply for commit
>>> privileges? this is becoming tedious)
>>
>> These changes should have gone in earlier, but you don't have commit
>> access, and review for 2.24 for myself has been tied up because of other
>> projects I had to handle. With 2.25 I hope you have commit access and get
>> these kinds of changes in earlier, they have lots of value and I enjoy
>> seeing responsible people fixing things.
...
>> Therefore I'm faced with the conclusion that I'd like to cut the branch
>> and _then_ commit these patches for you right away.
...
>
> I feel the same and it is just a timeframe issue.  Thanks for checking on
> this Carlos.

I'm fine with this.  It's a pretty old problem, it can wait another
release to be fixed.

zw

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 2/3] Minimize sysdeps code involved in defining major/minor/makedev.
  2016-08-01 20:55   ` Carlos O'Donell
@ 2016-08-01 21:28     ` Zack Weinberg
  2016-08-03 15:11       ` Carlos O'Donell
  0 siblings, 1 reply; 20+ messages in thread
From: Zack Weinberg @ 2016-08-01 21:28 UTC (permalink / raw)
  To: Carlos O'Donell; +Cc: GNU C Library, Adhemerval Zanella, Mike Frysinger

On Mon, Aug 1, 2016 at 4:55 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> (3) New symbols?
>
> Overall this looks good. I have only questions about why the new functions
> are going into the users namespace. Traditionally we put things into the
> implementation namespace, particularly to discourage users calling the
> functions directly. I would like the user to use the normal interfaces for
> calling this functionality.

I would agree if these were truly new functions, but they're not. They
already exist, with the user-namespace names, in every Linux-based
port.  `grep gnu_dev_ $(find sysdeps -name libc*.abilist)` and see for
yourself.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 1/3] Add utility macros for clang detection, and deprecation with messages.
  2016-08-01 14:20     ` Zack Weinberg
@ 2016-08-01 21:36       ` Paul Eggert
  0 siblings, 0 replies; 20+ messages in thread
From: Paul Eggert @ 2016-08-01 21:36 UTC (permalink / raw)
  To: Zack Weinberg
  Cc: GNU C Library, Adhemerval Zanella, Mike Frysinger, Carlos O'Donell

Zack Weinberg wrote:
> It might still be a good idea as part of a complete revision of
> sys/cdefs.h + features.h, subsuming _all_ of the GCC version checks
> into this mechanism and rendering it easily extensible to more
> different compilers -- which is what Roland wanted -- but that is a
> separate project, not appropriate for 2.24 at this stage, and I do not
> want to mission-creep _this_ patchset even to the extent of
> introducing the skeleton of the mechanism.

Yes, thanks, that all sounds good.

For what it's worth, the Emacs source code is already implementing something 
similar to what Roland and I were proposing (using different macro names).

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 2/3] Minimize sysdeps code involved in defining major/minor/makedev.
  2016-08-01 21:28     ` Zack Weinberg
@ 2016-08-03 15:11       ` Carlos O'Donell
  0 siblings, 0 replies; 20+ messages in thread
From: Carlos O'Donell @ 2016-08-03 15:11 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: GNU C Library, Adhemerval Zanella, Mike Frysinger

On 08/01/2016 05:28 PM, Zack Weinberg wrote:
> On Mon, Aug 1, 2016 at 4:55 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>> (3) New symbols?
>>
>> Overall this looks good. I have only questions about why the new functions
>> are going into the users namespace. Traditionally we put things into the
>> implementation namespace, particularly to discourage users calling the
>> functions directly. I would like the user to use the normal interfaces for
>> calling this functionality.
> 
> I would agree if these were truly new functions, but they're not. They
> already exist, with the user-namespace names, in every Linux-based
> port.  `grep gnu_dev_ $(find sysdeps -name libc*.abilist)` and see for
> yourself.
 
You are absolutely right. Sorry, I thought this had been inlined and the
functions not visible, but there they are at @@GLIBC_2.3.3 versions.

OK to checkin then.

-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 3/3] Deprecate inclusion of <sys/sysmacros.h> by <sys/types.h>
  2016-08-01 21:07   ` Carlos O'Donell
@ 2016-08-03 18:52     ` Zack Weinberg
       [not found]     ` <71ede440-4ace-161f-1848-4475fba05d0f@panix.com>
  1 sibling, 0 replies; 20+ messages in thread
From: Zack Weinberg @ 2016-08-03 18:52 UTC (permalink / raw)
  To: Carlos O'Donell; +Cc: GNU C Library

On Mon, Aug 1, 2016 at 5:07 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>> +  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.

I'm working on updating the patches again (deferring them to 2.25 may
mean that they go back to having to touch all the libc.abilist files,
feh) and I've created https://sourceware.org/glibc/wiki/Release/2.25
with a section about this, as you requested.  Please have a look.

zw

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 3/3] Deprecate inclusion of <sys/sysmacros.h> by <sys/types.h>
       [not found]       ` <7577c368-2850-c573-72d0-d65966d3bdc8@redhat.com>
@ 2016-08-03 20:06         ` Zack Weinberg
  2016-08-04 16:58           ` Carlos O'Donell
  0 siblings, 1 reply; 20+ messages in thread
From: Zack Weinberg @ 2016-08-03 20:06 UTC (permalink / raw)
  To: Carlos O'Donell; +Cc: GNU C Library

On Wed, Aug 3, 2016 at 11:12 AM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 08/02/2016 11:21 AM, Zack Weinberg wrote:
>> On 08/01/2016 05:07 PM, Carlos O'Donell wrote:
>>> 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.
>>
>> I think I have working commit access now.  Do you still intend to land
>> these patches yourself, or do you want me to do it?  Was my explanation
>> for the gnu_dev_* symbols adequate?
>
> Please commit the patches yourself as a first test of your new commit
> access.

I have committed these to master:

dbab6577c6684c62bd2521c1c29dc25c3cac966f Deprecate inclusion of
<sys/sysmacros.h> by <sys/types.h>
63eb8df85a17f7f966d4daa4cf44c8e956636a86 Minimize sysdeps code
involved in defining major/minor/makedev.
cab4d74b01320670f57dcf356ff89256f4d2fc12 Add utility macros for clang
detection, and deprecation with messages.

zw

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [2.24 PATCH 3/3] Deprecate inclusion of <sys/sysmacros.h> by <sys/types.h>
  2016-08-03 20:06         ` Zack Weinberg
@ 2016-08-04 16:58           ` Carlos O'Donell
  0 siblings, 0 replies; 20+ messages in thread
From: Carlos O'Donell @ 2016-08-04 16:58 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: GNU C Library

On 08/03/2016 04:06 PM, Zack Weinberg wrote:
> On Wed, Aug 3, 2016 at 11:12 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>> On 08/02/2016 11:21 AM, Zack Weinberg wrote:
>>> On 08/01/2016 05:07 PM, Carlos O'Donell wrote:
>>>> 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.
>>>
>>> I think I have working commit access now.  Do you still intend to land
>>> these patches yourself, or do you want me to do it?  Was my explanation
>>> for the gnu_dev_* symbols adequate?
>>
>> Please commit the patches yourself as a first test of your new commit
>> access.
> 
> I have committed these to master:
> 
> dbab6577c6684c62bd2521c1c29dc25c3cac966f Deprecate inclusion of
> <sys/sysmacros.h> by <sys/types.h>
> 63eb8df85a17f7f966d4daa4cf44c8e956636a86 Minimize sysdeps code
> involved in defining major/minor/makedev.
> cab4d74b01320670f57dcf356ff89256f4d2fc12 Add utility macros for clang
> detection, and deprecation with messages.

The wiki entry looks good. Thanks for committing the changes.

Sorry to hear that the solution doesn't work in some cases :-(

-- 
Cheers,
Carlos.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2016-08-04 16:58 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-31 20:09 [2.24 PATCH 0/3] sys/sysmacros.h and sys.types.h, revised^n Zack Weinberg
2016-07-31 20:09 ` [2.24 PATCH 2/3] Minimize sysdeps code involved in defining major/minor/makedev Zack Weinberg
2016-08-01 20:55   ` Carlos O'Donell
2016-08-01 21:28     ` Zack Weinberg
2016-08-03 15:11       ` Carlos O'Donell
2016-07-31 20:09 ` [2.24 PATCH 3/3] Deprecate inclusion of <sys/sysmacros.h> by <sys/types.h> Zack Weinberg
2016-08-01 21:07   ` Carlos O'Donell
2016-08-03 18:52     ` Zack Weinberg
     [not found]     ` <71ede440-4ace-161f-1848-4475fba05d0f@panix.com>
     [not found]       ` <7577c368-2850-c573-72d0-d65966d3bdc8@redhat.com>
2016-08-03 20:06         ` Zack Weinberg
2016-08-04 16:58           ` Carlos O'Donell
     [not found] ` <096b80f58b0361a9b34b2eb92e370b0592d15971.1469994984.git.zackw@panix.com>
2016-08-01  3:29   ` [2.24 PATCH 1/3] Add utility macros for clang detection, and deprecation with messages Paul Eggert
2016-08-01 14:20     ` Zack Weinberg
2016-08-01 21:36       ` Paul Eggert
2016-08-01 20:09   ` Carlos O'Donell
2016-08-01 20:19     ` Zack Weinberg
2016-08-01 20:46       ` Carlos O'Donell
2016-08-01 18:47 ` [2.24 PATCH 0/3] sys/sysmacros.h and sys.types.h, revised^n Carlos O'Donell
2016-08-01 20:38 ` Carlos O'Donell
2016-08-01 20:50   ` Adhemerval Zanella
2016-08-01 21:25     ` Zack Weinberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).