Deprecate union wait and remove support from wait functions [BZ #19613]

Deprecate union wait and remove support from wait functions [BZ #19613]

[Florian Weimer]

[-- Attachment #1: Type: text/plain, Size: 585 bytes --]

The overloading approach in the macros was incompatible with integer
expressions of a type different from int.  Applications using union wait
and these macros will have to migrate to the POSIX-specified int
status type.

This commit also removes the transparent union overloads from the
process wait functions.  As a result, application code which uses
union wait will receive compile-time warnings about pointer
compatibility.

This does not have ABI impact.  The functionality was deprecated in BSD
in the early 1990s.  I plan to remove union wait completely in glibc 2.25.

Florian

[-- Attachment #2: 0001-Deprecate-union-wait-and-remove-support-from-wait-fu.patch --]
[-- Type: text/x-patch, Size: 16045 bytes --]

2016-02-12  Florian Weimer  <fweimer@redhat.com>
 
	[BZ #19613]
	Deprecate union wait and remove union wait * support from W*
	macros in <sys/wait.h>.
	* bits/waitstatus.h (union wait): Mark as deprecated.
	* include/sys/wait.h (__wait, __wait3, __wait4): Use int * for the
	stat_loc argument.
	* posix/sys/wait.h (__WAIT_INT, __WAIT_STATUS)
	(__WAIT_STATUS_DEFN): Remove.
	(WEXITSTATUS, WTERMSIG, WSTOPSIG, WIFEXITED, WIFSIGNALED)
	(WIFSTOPPED, WIFCONTINUED, WCOREDUMP): Remove __WAIT_INT.
	(wait, wait3, wait4): Use int * for the stat_loc argument.
	* posix/wait.c (__wait): Likewise.
	* posix/wait3.c (__wait3): Likewise.
	* posix/wait4.c (__wait4): Likewise.
	* stdlib/stdlib.h (__WAIT_INT, __WAIT_STATUS)
	(__WAIT_STATUS_DEFN): Remove.
	(WEXITSTATUS, WTERMSIG, WSTOPSIG, WIFEXITED, WIFSIGNALED)
	(WIFSTOPPED, WIFCONTINUED): Remove __WAIT_INT.
	* sunrpc/key_call.c (key_call_keyenvoy): Use int instead of union
	wait.
	* sysdeps/mach/hurd/wait4.c (__wait4): Use int * for the stat_loc
	argument.
	* sysdeps/posix/wait.c (__libc_wait): Likewise.
	* sysdeps/posix/wait3.c (__wait3): Likewise.
	* sysdeps/unix/bsd/wait.c (__libc_wait): Likewise.
	* sysdeps/unix/bsd/wait3.c (__wait3): Likewise.
	* sysdeps/unix/bsd/waitpid.c (__waitpid): Remove cast.
	* sysdeps/unix/sysv/linux/wait.c (__libc_wait): Use int * for the
	stat_loc argument.
	* manual/process.texi (BSD Wait Functions): Remove union wait.

diff --git a/NEWS b/NEWS
index 93c09be..d58d732 100644
--- a/NEWS
+++ b/NEWS
@@ -73,6 +73,11 @@ Version 2.23
   C Library is GCC 4.7.  Older GCC versions, and non-GNU compilers, can
   still be used to compile programs using the GNU C Library.
 
+* The type union wait is deprecated.  It was obsoleted by 4.3+BSD in the
+  early 1990s.  Support for union wait *arguments in the W* status decoding
+  macros define <sys/wait.h> has been removed.  Application code should use
+  the int type instead of union wait.
+
 Security related changes:
 
 * The nan, nanf and nanl functions no longer have unbounded stack usage
diff --git a/bits/waitstatus.h b/bits/waitstatus.h
index 38c33bc..d6af076 100644
--- a/bits/waitstatus.h
+++ b/bits/waitstatus.h
@@ -63,7 +63,7 @@
 
 # include <endian.h>
 
-union wait
+union __attribute_deprecated__ wait
   {
     int w_status;
     struct
diff --git a/include/sys/wait.h b/include/sys/wait.h
index 9a38e61..5ac9cd6 100644
--- a/include/sys/wait.h
+++ b/include/sys/wait.h
@@ -9,10 +9,10 @@ libc_hidden_proto (__waitpid)
 extern int __waitid (idtype_t idtype, id_t id, siginfo_t *infop, int options);
 
 extern __pid_t __libc_wait (int *__stat_loc);
-extern __pid_t __wait (__WAIT_STATUS __stat_loc);
-extern __pid_t __wait3 (__WAIT_STATUS __stat_loc,
+extern __pid_t __wait (int *__stat_loc);
+extern __pid_t __wait3 (int *__stat_loc,
 			int __options, struct rusage * __usage);
-extern __pid_t __wait4 (__pid_t __pid, __WAIT_STATUS __stat_loc,
+extern __pid_t __wait4 (__pid_t __pid, int *__stat_loc,
 			int __options, struct rusage *__usage)
 			attribute_hidden;
 #endif
diff --git a/manual/process.texi b/manual/process.texi
index aa59040..25bdb8e 100644
--- a/manual/process.texi
+++ b/manual/process.texi
@@ -763,53 +763,17 @@ signal number of the signal that caused the child process to stop.
 
 
 @node BSD Wait Functions
-@section BSD Process Wait Functions
+@section BSD Process Wait Function
 
-@Theglibc{} also provides these related facilities for compatibility
-with BSD Unix.  BSD uses the @code{union wait} data type to represent
-status values rather than an @code{int}.  The two representations are
-actually interchangeable; they describe the same bit patterns.  @Theglibc{}
-defines macros such as @code{WEXITSTATUS} so that they will
-work on either kind of object, and the @code{wait} function is defined
-to accept either type of pointer as its @var{status-ptr} argument.
-
-These functions are declared in @file{sys/wait.h}.
+@Theglibc{} also provides the @code{wait3} function for compatibility
+with BSD.  This function is declared in @file{sys/wait.h}.  It is the
+predecessor to @code{wait4}, which is more flexible.  @code{wait3} is
+now obsolete.
 @pindex sys/wait.h
 
 @comment sys/wait.h
 @comment BSD
-@deftp {Data Type} {union wait}
-This data type represents program termination status values.  It has
-the following members:
-
-@table @code
-@item int w_termsig
-The value of this member is the same as that of the
-@code{WTERMSIG} macro.
-
-@item int w_coredump
-The value of this member is the same as that of the
-@code{WCOREDUMP} macro.
-
-@item int w_retcode
-The value of this member is the same as that of the
-@code{WEXITSTATUS} macro.
-
-@item int w_stopsig
-The value of this member is the same as that of the
-@code{WSTOPSIG} macro.
-@end table
-
-Instead of accessing these members directly, you should use the
-equivalent macros.
-@end deftp
-
-The @code{wait3} function is the predecessor to @code{wait4}, which is
-more flexible.  @code{wait3} is now obsolete.
-
-@comment sys/wait.h
-@comment BSD
-@deftypefun pid_t wait3 (union wait *@var{status-ptr}, int @var{options}, struct rusage *@var{usage})
+@deftypefun pid_t wait3 (int *@var{status-ptr}, int @var{options}, struct rusage *@var{usage})
 @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
 If @var{usage} is a null pointer, @code{wait3} is equivalent to
 @code{waitpid (-1, @var{status-ptr}, @var{options})}.
diff --git a/posix/sys/wait.h b/posix/sys/wait.h
index a673b41..acda43a 100644
--- a/posix/sys/wait.h
+++ b/posix/sys/wait.h
@@ -34,62 +34,23 @@ __BEGIN_DECLS
    bits to `waitpid', `wait3', and `wait4'.  */
 # include <bits/waitflags.h>
 
-# ifdef	__USE_MISC
-
-/* Lots of hair to allow traditional BSD use of `union wait'
-   as well as POSIX.1 use of `int' for the status word.  */
-
-#  if defined __GNUC__ && !defined __cplusplus
-#   define __WAIT_INT(status) \
-  (__extension__ (((union { __typeof(status) __in; int __i; }) \
-		   { .__in = (status) }).__i))
-#  else
-#   define __WAIT_INT(status)	(*(const int *) &(status))
-#  endif
-
-/* This is the type of the argument to `wait'.  The funky union
-   causes redeclarations with either `int *' or `union wait *' to be
-   allowed without complaint.  __WAIT_STATUS_DEFN is the type used in
-   the actual function definitions.  */
-
-#  if !defined __GNUC__ || __GNUC__ < 2 || defined __cplusplus
-#   define __WAIT_STATUS	void *
-#   define __WAIT_STATUS_DEFN	void *
-#  else
-/* This works in GCC 2.6.1 and later.  */
-typedef union
-  {
-    union wait *__uptr;
-    int *__iptr;
-  } __WAIT_STATUS __attribute__ ((__transparent_union__));
-#   define __WAIT_STATUS_DEFN	int *
-#  endif
-
-# else /* Don't use misc.  */
-
-#  define __WAIT_INT(status)	(status)
-#  define __WAIT_STATUS		int *
-#  define __WAIT_STATUS_DEFN	int *
-
-# endif /* Use misc.  */
-
 /* This will define all the `__W*' macros.  */
 # include <bits/waitstatus.h>
 
-# define WEXITSTATUS(status)	__WEXITSTATUS (__WAIT_INT (status))
-# define WTERMSIG(status)	__WTERMSIG (__WAIT_INT (status))
-# define WSTOPSIG(status)	__WSTOPSIG (__WAIT_INT (status))
-# define WIFEXITED(status)	__WIFEXITED (__WAIT_INT (status))
-# define WIFSIGNALED(status)	__WIFSIGNALED (__WAIT_INT (status))
-# define WIFSTOPPED(status)	__WIFSTOPPED (__WAIT_INT (status))
+# define WEXITSTATUS(status)	__WEXITSTATUS (status)
+# define WTERMSIG(status)	__WTERMSIG (status)
+# define WSTOPSIG(status)	__WSTOPSIG (status)
+# define WIFEXITED(status)	__WIFEXITED (status)
+# define WIFSIGNALED(status)	__WIFSIGNALED (status)
+# define WIFSTOPPED(status)	__WIFSTOPPED (status)
 # ifdef __WIFCONTINUED
-#  define WIFCONTINUED(status)	__WIFCONTINUED (__WAIT_INT (status))
+#  define WIFCONTINUED(status)	__WIFCONTINUED (status)
 # endif
 #endif	/* <stdlib.h> not included.  */
 
 #ifdef	__USE_MISC
 # define WCOREFLAG		__WCOREFLAG
-# define WCOREDUMP(status)	__WCOREDUMP (__WAIT_INT (status))
+# define WCOREDUMP(status)	__WCOREDUMP (status)
 # define W_EXITCODE(ret, sig)	__W_EXITCODE (ret, sig)
 # define W_STOPCODE(sig)	__W_STOPCODE (sig)
 #endif
@@ -110,7 +71,7 @@ typedef enum
 
    This function is a cancellation point and therefore not marked with
    __THROW.  */
-extern __pid_t wait (__WAIT_STATUS __stat_loc);
+extern __pid_t wait (int *__stat_loc);
 
 #ifdef	__USE_MISC
 /* Special values for the PID argument to `waitpid' and `wait4'.  */
@@ -170,13 +131,13 @@ struct rusage;
    nil, store information about the child's resource usage there.  If the
    WUNTRACED bit is set in OPTIONS, return status for stopped children;
    otherwise don't.  */
-extern __pid_t wait3 (__WAIT_STATUS __stat_loc, int __options,
+extern __pid_t wait3 (int *__stat_loc, int __options,
 		      struct rusage * __usage) __THROWNL;
 #endif
 
 #ifdef __USE_MISC
 /* PID is like waitpid.  Other args are like wait3.  */
-extern __pid_t wait4 (__pid_t __pid, __WAIT_STATUS __stat_loc, int __options,
+extern __pid_t wait4 (__pid_t __pid, int *__stat_loc, int __options,
 		      struct rusage *__usage) __THROWNL;
 #endif /* Use misc.  */
 
diff --git a/posix/wait.c b/posix/wait.c
index 3abf30e..c49375b 100644
--- a/posix/wait.c
+++ b/posix/wait.c
@@ -21,7 +21,7 @@
 /* Wait for a child to die.  When one does, put its status in *STAT_LOC
    and return its process ID.  For errors, return (pid_t) -1.  */
 __pid_t
-__wait (__WAIT_STATUS_DEFN stat_loc)
+__wait (int *stat_loc)
 {
   __set_errno (ENOSYS);
   return -1;
diff --git a/posix/wait3.c b/posix/wait3.c
index 356839a..e8c2930 100644
--- a/posix/wait3.c
+++ b/posix/wait3.c
@@ -25,7 +25,7 @@
    there.  If the WUNTRACED bit is set in OPTIONS, return status for stopped
    children; otherwise don't.  */
 pid_t
-__wait3 (__WAIT_STATUS_DEFN stat_loc, int options, struct rusage *usage)
+__wait3 (int *stat_loc, int options, struct rusage *usage)
 {
   if ((options & ~(WNOHANG|WUNTRACED)) != 0)
     {
diff --git a/posix/wait4.c b/posix/wait4.c
index e5b0376..4137617 100644
--- a/posix/wait4.c
+++ b/posix/wait4.c
@@ -20,8 +20,7 @@
 #include <errno.h>
 
 pid_t
-__wait4 (__pid_t pid, __WAIT_STATUS stat_loc, int options,
-	 struct rusage *usage)
+__wait4 (__pid_t pid, int *stat_loc, int options, struct rusage *usage)
 {
   __set_errno (ENOSYS);
   return (pid_t) -1;
diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
index cc77708..d0c78fa 100644
--- a/stdlib/stdlib.h
+++ b/stdlib/stdlib.h
@@ -41,54 +41,15 @@ __BEGIN_DECLS
 # include <bits/waitflags.h>
 # include <bits/waitstatus.h>
 
-# ifdef __USE_MISC
-
-/* Lots of hair to allow traditional BSD use of `union wait'
-   as well as POSIX.1 use of `int' for the status word.  */
-
-#  if defined __GNUC__ && !defined __cplusplus
-#   define __WAIT_INT(status) \
-  (__extension__ (((union { __typeof(status) __in; int __i; }) \
-		   { .__in = (status) }).__i))
-#  else
-#   define __WAIT_INT(status)	(*(int *) &(status))
-#  endif
-
-/* This is the type of the argument to `wait'.  The funky union
-   causes redeclarations with either `int *' or `union wait *' to be
-   allowed without complaint.  __WAIT_STATUS_DEFN is the type used in
-   the actual function definitions.  */
-
-#  if !defined __GNUC__ || __GNUC__ < 2 || defined __cplusplus
-#   define __WAIT_STATUS	void *
-#   define __WAIT_STATUS_DEFN	void *
-#  else
-/* This works in GCC 2.6.1 and later.  */
-typedef union
-  {
-    union wait *__uptr;
-    int *__iptr;
-  } __WAIT_STATUS __attribute__ ((__transparent_union__));
-#   define __WAIT_STATUS_DEFN	int *
-#  endif
-
-# else /* Don't use misc.  */
-
-#  define __WAIT_INT(status)	(status)
-#  define __WAIT_STATUS		int *
-#  define __WAIT_STATUS_DEFN	int *
-
-# endif /* Use misc.  */
-
 /* Define the macros <sys/wait.h> also would define this way.  */
-# define WEXITSTATUS(status)	__WEXITSTATUS (__WAIT_INT (status))
-# define WTERMSIG(status)	__WTERMSIG (__WAIT_INT (status))
-# define WSTOPSIG(status)	__WSTOPSIG (__WAIT_INT (status))
-# define WIFEXITED(status)	__WIFEXITED (__WAIT_INT (status))
-# define WIFSIGNALED(status)	__WIFSIGNALED (__WAIT_INT (status))
-# define WIFSTOPPED(status)	__WIFSTOPPED (__WAIT_INT (status))
+# define WEXITSTATUS(status)	__WEXITSTATUS (status)
+# define WTERMSIG(status)	__WTERMSIG (status)
+# define WSTOPSIG(status)	__WSTOPSIG (status)
+# define WIFEXITED(status)	__WIFEXITED (status)
+# define WIFSIGNALED(status)	__WIFSIGNALED (status)
+# define WIFSTOPPED(status)	__WIFSTOPPED (status)
 # ifdef __WIFCONTINUED
-#  define WIFCONTINUED(status)	__WIFCONTINUED (__WAIT_INT (status))
+#  define WIFCONTINUED(status)	__WIFCONTINUED (status)
 # endif
 #endif	/* X/Open or XPG7 and <sys/wait.h> not included.  */
 
diff --git a/sunrpc/key_call.c b/sunrpc/key_call.c
index 7ecf6fb..7880ab9 100644
--- a/sunrpc/key_call.c
+++ b/sunrpc/key_call.c
@@ -304,7 +304,7 @@ key_call_keyenvoy (u_long proc, xdrproc_t xdr_arg, char *arg,
   FILE *fargs;
   FILE *frslt;
   sigset_t oldmask, mask;
-  union wait status;
+  int status;
   int pid;
   int success;
   uid_t ruid;
diff --git a/sysdeps/mach/hurd/wait4.c b/sysdeps/mach/hurd/wait4.c
index 3bc9fa8..f392a98 100644
--- a/sysdeps/mach/hurd/wait4.c
+++ b/sysdeps/mach/hurd/wait4.c
@@ -22,8 +22,7 @@
 #include <hurd/port.h>
 
 pid_t
-__wait4 (pid_t pid, __WAIT_STATUS_DEFN stat_loc, int options,
-	 struct rusage *usage)
+__wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
 {
   pid_t dead;
   error_t err;
diff --git a/sysdeps/posix/wait.c b/sysdeps/posix/wait.c
index 7f1d71a..210ece8 100644
--- a/sysdeps/posix/wait.c
+++ b/sysdeps/posix/wait.c
@@ -21,7 +21,7 @@
 /* Wait for a child to die.  When one does, put its status in *STAT_LOC
    and return its process ID.  For errors, return (pid_t) -1.  */
 __pid_t
-__libc_wait (__WAIT_STATUS_DEFN stat_loc)
+__libc_wait (int *stat_loc)
 {
   return __waitpid (WAIT_ANY, (int *) stat_loc, 0);
 }
diff --git a/sysdeps/posix/wait3.c b/sysdeps/posix/wait3.c
index 2e76892..cf43d97 100644
--- a/sysdeps/posix/wait3.c
+++ b/sysdeps/posix/wait3.c
@@ -26,7 +26,7 @@
    there.  If the WUNTRACED bit is set in OPTIONS, return status for stopped
    children; otherwise don't.  */
 pid_t
-__wait3 (__WAIT_STATUS stat_loc, int options, struct rusage *usage)
+__wait3 (int *stat_loc, int options, struct rusage *usage)
 {
   if (usage != NULL)
     {
diff --git a/sysdeps/unix/bsd/wait.c b/sysdeps/unix/bsd/wait.c
index 31de60e..a9e29f2 100644
--- a/sysdeps/unix/bsd/wait.c
+++ b/sysdeps/unix/bsd/wait.c
@@ -23,7 +23,7 @@
 /* Wait for a child to die.  When one does, put its status in *STAT_LOC
    and return its process ID.  For errors, return (pid_t) -1.  */
 __pid_t
-__libc_wait (__WAIT_STATUS_DEFN stat_loc)
+__libc_wait (int *stat_loc)
 {
   return __wait4 (WAIT_ANY, stat_loc, 0, (struct rusage *) NULL);
 }
diff --git a/sysdeps/unix/bsd/wait3.c b/sysdeps/unix/bsd/wait3.c
index 3f95ac7..ee33a65 100644
--- a/sysdeps/unix/bsd/wait3.c
+++ b/sysdeps/unix/bsd/wait3.c
@@ -25,7 +25,7 @@
    there.  If the WUNTRACED bit is set in OPTIONS, return status for stopped
    children; otherwise don't.  */
 pid_t
-__wait3 (__WAIT_STATUS stat_loc, int options, struct rusage *usage)
+__wait3 (int *stat_loc, int options, struct rusage *usage)
 {
   return __wait4 (WAIT_ANY, stat_loc, options, usage);
 }
diff --git a/sysdeps/unix/bsd/waitpid.c b/sysdeps/unix/bsd/waitpid.c
index 083c686..cfe5614 100644
--- a/sysdeps/unix/bsd/waitpid.c
+++ b/sysdeps/unix/bsd/waitpid.c
@@ -35,7 +35,7 @@
 pid_t
 __waitpid (pid_t pid, int *stat_loc, int options)
 {
-  return __wait4 (pid, (union wait *) stat_loc, options, NULL);
+  return __wait4 (pid, stat_loc, options, NULL);
 }
 
 libc_hidden_def (__waitpid)
diff --git a/sysdeps/unix/sysv/linux/wait.c b/sysdeps/unix/sysv/linux/wait.c
index 71f9044..9b7c9c9 100644
--- a/sysdeps/unix/sysv/linux/wait.c
+++ b/sysdeps/unix/sysv/linux/wait.c
@@ -24,7 +24,7 @@
 /* Wait for a child to die.  When one does, put its status in *STAT_LOC
    and return its process ID.  For errors, return (pid_t) -1.  */
 pid_t
-__libc_wait (__WAIT_STATUS_DEFN stat_loc)
+__libc_wait (int *stat_loc)
 {
   pid_t result = SYSCALL_CANCEL (wait4, WAIT_ANY, stat_loc, 0,
 				 (struct rusage *) NULL);

Re: Deprecate union wait and remove support from wait functions [BZ #19613]

[Florian Weimer]

On 02/12/2016 05:26 PM, Florian Weimer wrote:
> The overloading approach in the macros was incompatible with integer
> expressions of a type different from int.  Applications using union wait
> and these macros will have to migrate to the POSIX-specified int
> status type.
> 
> This commit also removes the transparent union overloads from the
> process wait functions.  As a result, application code which uses
> union wait will receive compile-time warnings about pointer
> compatibility.
> 
> This does not have ABI impact.  The functionality was deprecated in BSD
> in the early 1990s.  I plan to remove union wait completely in glibc 2.25.

Ping?

I'd like to have some buy-in for this API change.

Thanks,
Florian

Re: Deprecate union wait and remove support from wait functions [BZ #19613]

[Paul Eggert]

On 03/07/2016 12:38 PM, Florian Weimer wrote:
> I'd like to have some buy-in for this API change.

I have read the patch and it looks good to me. Thanks for persevering on 
this.

Re: Deprecate union wait and remove support from wait functions [BZ #19613]

[Roland McGrath]

I don't see a rationale for keeping union wait at all if it can be longer
be used with the wait* functions.  If we need staged deprecation, then it
should be a staged deprecation of all the features (with the possible
exception of W* macros accepting 'union wait' argument values, which was
always a GNU extension over the 4.3BSD API that specified 'union wait').
Personally I'm OK with just removing it entirely in one cycle.

> 	[BZ #19613]
> 	Deprecate union wait and remove union wait * support from W*
> 	macros in <sys/wait.h>.

The W* macros support arguments of type 'union wait', not 'union wait *'.

> 	* sunrpc/key_call.c (key_call_keyenvoy): Use int instead of union
> 	wait.

This one change could and should go in separately first.

> diff --git a/sunrpc/key_call.c b/sunrpc/key_call.c
> index 7ecf6fb..7880ab9 100644
> --- a/sunrpc/key_call.c
> +++ b/sunrpc/key_call.c
> @@ -304,7 +304,7 @@ key_call_keyenvoy (u_long proc, xdrproc_t xdr_arg, char *arg,
>    FILE *fargs;
>    FILE *frslt;
>    sigset_t oldmask, mask;
> -  union wait status;
> +  int status;
>    int pid;
>    int success;
>    uid_t ruid;

This is insufficient.  You need to adjust the use of STATUS as well.  This
function is under #ifndef SO_PASSCRED so you'd probably have to do a Hurd
build to see actually get built.  If you hand-hacked out the #ifndef for
purposes of compile testing on a Linux configuration, then that would be
good enough for the change to go in.


Thanks,
Roland

[PATCH v2] Remove union wait (was: Re: Deprecate union wait and remove support from wait functions [BZ #19613])

[Florian Weimer]

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]

On 03/07/2016 11:55 PM, Roland McGrath wrote:
> I don't see a rationale for keeping union wait at all if it can be longer
> be used with the wait* functions.

I think it can, you just get a pointer conversion warning, and have to
use the union members (which are the original interface) instead of the
macro wrappers.

> If we need staged deprecation, then it
> should be a staged deprecation of all the features (with the possible
> exception of W* macros accepting 'union wait' argument values, which was
> always a GNU extension over the 4.3BSD API that specified 'union wait').
> Personally I'm OK with just removing it entirely in one cycle.

Okay, let's do it this way.  New patch attached.

>> 	* sunrpc/key_call.c (key_call_keyenvoy): Use int instead of union
>> 	wait.
> 
> This one change could and should go in separately first.

Done.

Florian

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Remove-union-wait-BZ-19613.patch --]
[-- Type: text/x-patch; name="0001-Remove-union-wait-BZ-19613.patch", Size: 17238 bytes --]

2016-03-08  Florian Weimer  <fweimer@redhat.com>

	[BZ #19613]
	Remove union wait.
	* bits/waitstatus.h (union wait, w_termsig, w_coredump, w_retcode)
	(w_stopsig, w_stopval): Remove.
	* include/sys/wait.h (__wait, __wait3, __wait4): Use int * for the
	stat_loc argument.
	* posix/sys/wait.h (__WAIT_INT, __WAIT_STATUS)
	(__WAIT_STATUS_DEFN): Remove.
	(WEXITSTATUS, WTERMSIG, WSTOPSIG, WIFEXITED, WIFSIGNALED)
	(WIFSTOPPED, WIFCONTINUED, WCOREDUMP): Remove __WAIT_INT.
	(wait, wait3, wait4): Use int * for the stat_loc argument.
	* posix/wait.c (__wait): Likewise.
	* posix/wait3.c (__wait3): Likewise.
	* posix/wait4.c (__wait4): Likewise.
	* stdlib/stdlib.h (__WAIT_INT, __WAIT_STATUS)
	(__WAIT_STATUS_DEFN): Remove.
	(WEXITSTATUS, WTERMSIG, WSTOPSIG, WIFEXITED, WIFSIGNALED)
	(WIFSTOPPED, WIFCONTINUED): Remove __WAIT_INT.
	* sysdeps/mach/hurd/wait4.c (__wait4): Use int * for the stat_loc
	argument.
	* sysdeps/posix/wait.c (__libc_wait): Likewise.
	* sysdeps/posix/wait3.c (__wait3): Likewise.
	* sysdeps/unix/bsd/wait.c (__libc_wait): Likewise.
	* sysdeps/unix/bsd/wait3.c (__wait3): Likewise.
	* sysdeps/unix/bsd/waitpid.c (__waitpid): Remove cast.
	* sysdeps/unix/sysv/linux/wait.c (__libc_wait): Use int * for the
	stat_loc argument.
	* manual/process.texi (BSD Wait Functions): Remove union wait.

diff --git a/NEWS b/NEWS
index a06a42f..ed17199 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,10 @@ Version 2.24
 * The readdir_r and readdir64_r functions have been deprecated.  It is
   recommended to use readdir and readdir64 instead.
 
+* The type union wait has been removed.  It was obsoleted by 4.3+BSD in the
+  early 1990s.  Application code should use the int type instead of union
+  wait.
+
 Security related changes:
 
   [Add security related changes here]
diff --git a/bits/waitstatus.h b/bits/waitstatus.h
index 38c33bc..069ce4b 100644
--- a/bits/waitstatus.h
+++ b/bits/waitstatus.h
@@ -57,49 +57,3 @@
 #define	__W_STOPCODE(sig)	((sig) << 8 | 0x7f)
 #define __W_CONTINUED		0xffff
 #define	__WCOREFLAG		0x80
-
-
-#ifdef	__USE_MISC
-
-# include <endian.h>
-
-union wait
-  {
-    int w_status;
-    struct
-      {
-# if	__BYTE_ORDER == __LITTLE_ENDIAN
-	unsigned int __w_termsig:7; /* Terminating signal.  */
-	unsigned int __w_coredump:1; /* Set if dumped core.  */
-	unsigned int __w_retcode:8; /* Return code if exited normally.  */
-	unsigned int:16;
-# endif				/* Little endian.  */
-# if	__BYTE_ORDER == __BIG_ENDIAN
-	unsigned int:16;
-	unsigned int __w_retcode:8;
-	unsigned int __w_coredump:1;
-	unsigned int __w_termsig:7;
-# endif				/* Big endian.  */
-      } __wait_terminated;
-    struct
-      {
-# if	__BYTE_ORDER == __LITTLE_ENDIAN
-	unsigned int __w_stopval:8; /* W_STOPPED if stopped.  */
-	unsigned int __w_stopsig:8; /* Stopping signal.  */
-	unsigned int:16;
-# endif				/* Little endian.  */
-# if	__BYTE_ORDER == __BIG_ENDIAN
-	unsigned int:16;
-	unsigned int __w_stopsig:8; /* Stopping signal.  */
-	unsigned int __w_stopval:8; /* W_STOPPED if stopped.  */
-# endif				/* Big endian.  */
-      } __wait_stopped;
-  };
-
-# define w_termsig	__wait_terminated.__w_termsig
-# define w_coredump	__wait_terminated.__w_coredump
-# define w_retcode	__wait_terminated.__w_retcode
-# define w_stopsig	__wait_stopped.__w_stopsig
-# define w_stopval	__wait_stopped.__w_stopval
-
-#endif	/* Use misc.  */
diff --git a/include/sys/wait.h b/include/sys/wait.h
index 9a38e61..5ac9cd6 100644
--- a/include/sys/wait.h
+++ b/include/sys/wait.h
@@ -9,10 +9,10 @@ libc_hidden_proto (__waitpid)
 extern int __waitid (idtype_t idtype, id_t id, siginfo_t *infop, int options);
 
 extern __pid_t __libc_wait (int *__stat_loc);
-extern __pid_t __wait (__WAIT_STATUS __stat_loc);
-extern __pid_t __wait3 (__WAIT_STATUS __stat_loc,
+extern __pid_t __wait (int *__stat_loc);
+extern __pid_t __wait3 (int *__stat_loc,
 			int __options, struct rusage * __usage);
-extern __pid_t __wait4 (__pid_t __pid, __WAIT_STATUS __stat_loc,
+extern __pid_t __wait4 (__pid_t __pid, int *__stat_loc,
 			int __options, struct rusage *__usage)
 			attribute_hidden;
 #endif
diff --git a/manual/process.texi b/manual/process.texi
index aa59040..25bdb8e 100644
--- a/manual/process.texi
+++ b/manual/process.texi
@@ -763,53 +763,17 @@ signal number of the signal that caused the child process to stop.
 
 
 @node BSD Wait Functions
-@section BSD Process Wait Functions
+@section BSD Process Wait Function
 
-@Theglibc{} also provides these related facilities for compatibility
-with BSD Unix.  BSD uses the @code{union wait} data type to represent
-status values rather than an @code{int}.  The two representations are
-actually interchangeable; they describe the same bit patterns.  @Theglibc{}
-defines macros such as @code{WEXITSTATUS} so that they will
-work on either kind of object, and the @code{wait} function is defined
-to accept either type of pointer as its @var{status-ptr} argument.
-
-These functions are declared in @file{sys/wait.h}.
+@Theglibc{} also provides the @code{wait3} function for compatibility
+with BSD.  This function is declared in @file{sys/wait.h}.  It is the
+predecessor to @code{wait4}, which is more flexible.  @code{wait3} is
+now obsolete.
 @pindex sys/wait.h
 
 @comment sys/wait.h
 @comment BSD
-@deftp {Data Type} {union wait}
-This data type represents program termination status values.  It has
-the following members:
-
-@table @code
-@item int w_termsig
-The value of this member is the same as that of the
-@code{WTERMSIG} macro.
-
-@item int w_coredump
-The value of this member is the same as that of the
-@code{WCOREDUMP} macro.
-
-@item int w_retcode
-The value of this member is the same as that of the
-@code{WEXITSTATUS} macro.
-
-@item int w_stopsig
-The value of this member is the same as that of the
-@code{WSTOPSIG} macro.
-@end table
-
-Instead of accessing these members directly, you should use the
-equivalent macros.
-@end deftp
-
-The @code{wait3} function is the predecessor to @code{wait4}, which is
-more flexible.  @code{wait3} is now obsolete.
-
-@comment sys/wait.h
-@comment BSD
-@deftypefun pid_t wait3 (union wait *@var{status-ptr}, int @var{options}, struct rusage *@var{usage})
+@deftypefun pid_t wait3 (int *@var{status-ptr}, int @var{options}, struct rusage *@var{usage})
 @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
 If @var{usage} is a null pointer, @code{wait3} is equivalent to
 @code{waitpid (-1, @var{status-ptr}, @var{options})}.
diff --git a/posix/sys/wait.h b/posix/sys/wait.h
index a673b41..acda43a 100644
--- a/posix/sys/wait.h
+++ b/posix/sys/wait.h
@@ -34,62 +34,23 @@ __BEGIN_DECLS
    bits to `waitpid', `wait3', and `wait4'.  */
 # include <bits/waitflags.h>
 
-# ifdef	__USE_MISC
-
-/* Lots of hair to allow traditional BSD use of `union wait'
-   as well as POSIX.1 use of `int' for the status word.  */
-
-#  if defined __GNUC__ && !defined __cplusplus
-#   define __WAIT_INT(status) \
-  (__extension__ (((union { __typeof(status) __in; int __i; }) \
-		   { .__in = (status) }).__i))
-#  else
-#   define __WAIT_INT(status)	(*(const int *) &(status))
-#  endif
-
-/* This is the type of the argument to `wait'.  The funky union
-   causes redeclarations with either `int *' or `union wait *' to be
-   allowed without complaint.  __WAIT_STATUS_DEFN is the type used in
-   the actual function definitions.  */
-
-#  if !defined __GNUC__ || __GNUC__ < 2 || defined __cplusplus
-#   define __WAIT_STATUS	void *
-#   define __WAIT_STATUS_DEFN	void *
-#  else
-/* This works in GCC 2.6.1 and later.  */
-typedef union
-  {
-    union wait *__uptr;
-    int *__iptr;
-  } __WAIT_STATUS __attribute__ ((__transparent_union__));
-#   define __WAIT_STATUS_DEFN	int *
-#  endif
-
-# else /* Don't use misc.  */
-
-#  define __WAIT_INT(status)	(status)
-#  define __WAIT_STATUS		int *
-#  define __WAIT_STATUS_DEFN	int *
-
-# endif /* Use misc.  */
-
 /* This will define all the `__W*' macros.  */
 # include <bits/waitstatus.h>
 
-# define WEXITSTATUS(status)	__WEXITSTATUS (__WAIT_INT (status))
-# define WTERMSIG(status)	__WTERMSIG (__WAIT_INT (status))
-# define WSTOPSIG(status)	__WSTOPSIG (__WAIT_INT (status))
-# define WIFEXITED(status)	__WIFEXITED (__WAIT_INT (status))
-# define WIFSIGNALED(status)	__WIFSIGNALED (__WAIT_INT (status))
-# define WIFSTOPPED(status)	__WIFSTOPPED (__WAIT_INT (status))
+# define WEXITSTATUS(status)	__WEXITSTATUS (status)
+# define WTERMSIG(status)	__WTERMSIG (status)
+# define WSTOPSIG(status)	__WSTOPSIG (status)
+# define WIFEXITED(status)	__WIFEXITED (status)
+# define WIFSIGNALED(status)	__WIFSIGNALED (status)
+# define WIFSTOPPED(status)	__WIFSTOPPED (status)
 # ifdef __WIFCONTINUED
-#  define WIFCONTINUED(status)	__WIFCONTINUED (__WAIT_INT (status))
+#  define WIFCONTINUED(status)	__WIFCONTINUED (status)
 # endif
 #endif	/* <stdlib.h> not included.  */
 
 #ifdef	__USE_MISC
 # define WCOREFLAG		__WCOREFLAG
-# define WCOREDUMP(status)	__WCOREDUMP (__WAIT_INT (status))
+# define WCOREDUMP(status)	__WCOREDUMP (status)
 # define W_EXITCODE(ret, sig)	__W_EXITCODE (ret, sig)
 # define W_STOPCODE(sig)	__W_STOPCODE (sig)
 #endif
@@ -110,7 +71,7 @@ typedef enum
 
    This function is a cancellation point and therefore not marked with
    __THROW.  */
-extern __pid_t wait (__WAIT_STATUS __stat_loc);
+extern __pid_t wait (int *__stat_loc);
 
 #ifdef	__USE_MISC
 /* Special values for the PID argument to `waitpid' and `wait4'.  */
@@ -170,13 +131,13 @@ struct rusage;
    nil, store information about the child's resource usage there.  If the
    WUNTRACED bit is set in OPTIONS, return status for stopped children;
    otherwise don't.  */
-extern __pid_t wait3 (__WAIT_STATUS __stat_loc, int __options,
+extern __pid_t wait3 (int *__stat_loc, int __options,
 		      struct rusage * __usage) __THROWNL;
 #endif
 
 #ifdef __USE_MISC
 /* PID is like waitpid.  Other args are like wait3.  */
-extern __pid_t wait4 (__pid_t __pid, __WAIT_STATUS __stat_loc, int __options,
+extern __pid_t wait4 (__pid_t __pid, int *__stat_loc, int __options,
 		      struct rusage *__usage) __THROWNL;
 #endif /* Use misc.  */
 
diff --git a/posix/wait.c b/posix/wait.c
index 3abf30e..c49375b 100644
--- a/posix/wait.c
+++ b/posix/wait.c
@@ -21,7 +21,7 @@
 /* Wait for a child to die.  When one does, put its status in *STAT_LOC
    and return its process ID.  For errors, return (pid_t) -1.  */
 __pid_t
-__wait (__WAIT_STATUS_DEFN stat_loc)
+__wait (int *stat_loc)
 {
   __set_errno (ENOSYS);
   return -1;
diff --git a/posix/wait3.c b/posix/wait3.c
index 356839a..e8c2930 100644
--- a/posix/wait3.c
+++ b/posix/wait3.c
@@ -25,7 +25,7 @@
    there.  If the WUNTRACED bit is set in OPTIONS, return status for stopped
    children; otherwise don't.  */
 pid_t
-__wait3 (__WAIT_STATUS_DEFN stat_loc, int options, struct rusage *usage)
+__wait3 (int *stat_loc, int options, struct rusage *usage)
 {
   if ((options & ~(WNOHANG|WUNTRACED)) != 0)
     {
diff --git a/posix/wait4.c b/posix/wait4.c
index e5b0376..4137617 100644
--- a/posix/wait4.c
+++ b/posix/wait4.c
@@ -20,8 +20,7 @@
 #include <errno.h>
 
 pid_t
-__wait4 (__pid_t pid, __WAIT_STATUS stat_loc, int options,
-	 struct rusage *usage)
+__wait4 (__pid_t pid, int *stat_loc, int options, struct rusage *usage)
 {
   __set_errno (ENOSYS);
   return (pid_t) -1;
diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
index cc77708..d0c78fa 100644
--- a/stdlib/stdlib.h
+++ b/stdlib/stdlib.h
@@ -41,54 +41,15 @@ __BEGIN_DECLS
 # include <bits/waitflags.h>
 # include <bits/waitstatus.h>
 
-# ifdef __USE_MISC
-
-/* Lots of hair to allow traditional BSD use of `union wait'
-   as well as POSIX.1 use of `int' for the status word.  */
-
-#  if defined __GNUC__ && !defined __cplusplus
-#   define __WAIT_INT(status) \
-  (__extension__ (((union { __typeof(status) __in; int __i; }) \
-		   { .__in = (status) }).__i))
-#  else
-#   define __WAIT_INT(status)	(*(int *) &(status))
-#  endif
-
-/* This is the type of the argument to `wait'.  The funky union
-   causes redeclarations with either `int *' or `union wait *' to be
-   allowed without complaint.  __WAIT_STATUS_DEFN is the type used in
-   the actual function definitions.  */
-
-#  if !defined __GNUC__ || __GNUC__ < 2 || defined __cplusplus
-#   define __WAIT_STATUS	void *
-#   define __WAIT_STATUS_DEFN	void *
-#  else
-/* This works in GCC 2.6.1 and later.  */
-typedef union
-  {
-    union wait *__uptr;
-    int *__iptr;
-  } __WAIT_STATUS __attribute__ ((__transparent_union__));
-#   define __WAIT_STATUS_DEFN	int *
-#  endif
-
-# else /* Don't use misc.  */
-
-#  define __WAIT_INT(status)	(status)
-#  define __WAIT_STATUS		int *
-#  define __WAIT_STATUS_DEFN	int *
-
-# endif /* Use misc.  */
-
 /* Define the macros <sys/wait.h> also would define this way.  */
-# define WEXITSTATUS(status)	__WEXITSTATUS (__WAIT_INT (status))
-# define WTERMSIG(status)	__WTERMSIG (__WAIT_INT (status))
-# define WSTOPSIG(status)	__WSTOPSIG (__WAIT_INT (status))
-# define WIFEXITED(status)	__WIFEXITED (__WAIT_INT (status))
-# define WIFSIGNALED(status)	__WIFSIGNALED (__WAIT_INT (status))
-# define WIFSTOPPED(status)	__WIFSTOPPED (__WAIT_INT (status))
+# define WEXITSTATUS(status)	__WEXITSTATUS (status)
+# define WTERMSIG(status)	__WTERMSIG (status)
+# define WSTOPSIG(status)	__WSTOPSIG (status)
+# define WIFEXITED(status)	__WIFEXITED (status)
+# define WIFSIGNALED(status)	__WIFSIGNALED (status)
+# define WIFSTOPPED(status)	__WIFSTOPPED (status)
 # ifdef __WIFCONTINUED
-#  define WIFCONTINUED(status)	__WIFCONTINUED (__WAIT_INT (status))
+#  define WIFCONTINUED(status)	__WIFCONTINUED (status)
 # endif
 #endif	/* X/Open or XPG7 and <sys/wait.h> not included.  */
 
diff --git a/sysdeps/mach/hurd/wait4.c b/sysdeps/mach/hurd/wait4.c
index 3bc9fa8..f392a98 100644
--- a/sysdeps/mach/hurd/wait4.c
+++ b/sysdeps/mach/hurd/wait4.c
@@ -22,8 +22,7 @@
 #include <hurd/port.h>
 
 pid_t
-__wait4 (pid_t pid, __WAIT_STATUS_DEFN stat_loc, int options,
-	 struct rusage *usage)
+__wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
 {
   pid_t dead;
   error_t err;
diff --git a/sysdeps/posix/wait.c b/sysdeps/posix/wait.c
index 7f1d71a..210ece8 100644
--- a/sysdeps/posix/wait.c
+++ b/sysdeps/posix/wait.c
@@ -21,7 +21,7 @@
 /* Wait for a child to die.  When one does, put its status in *STAT_LOC
    and return its process ID.  For errors, return (pid_t) -1.  */
 __pid_t
-__libc_wait (__WAIT_STATUS_DEFN stat_loc)
+__libc_wait (int *stat_loc)
 {
   return __waitpid (WAIT_ANY, (int *) stat_loc, 0);
 }
diff --git a/sysdeps/posix/wait3.c b/sysdeps/posix/wait3.c
index 2e76892..cf43d97 100644
--- a/sysdeps/posix/wait3.c
+++ b/sysdeps/posix/wait3.c
@@ -26,7 +26,7 @@
    there.  If the WUNTRACED bit is set in OPTIONS, return status for stopped
    children; otherwise don't.  */
 pid_t
-__wait3 (__WAIT_STATUS stat_loc, int options, struct rusage *usage)
+__wait3 (int *stat_loc, int options, struct rusage *usage)
 {
   if (usage != NULL)
     {
diff --git a/sysdeps/unix/bsd/wait.c b/sysdeps/unix/bsd/wait.c
index 31de60e..a9e29f2 100644
--- a/sysdeps/unix/bsd/wait.c
+++ b/sysdeps/unix/bsd/wait.c
@@ -23,7 +23,7 @@
 /* Wait for a child to die.  When one does, put its status in *STAT_LOC
    and return its process ID.  For errors, return (pid_t) -1.  */
 __pid_t
-__libc_wait (__WAIT_STATUS_DEFN stat_loc)
+__libc_wait (int *stat_loc)
 {
   return __wait4 (WAIT_ANY, stat_loc, 0, (struct rusage *) NULL);
 }
diff --git a/sysdeps/unix/bsd/wait3.c b/sysdeps/unix/bsd/wait3.c
index 3f95ac7..ee33a65 100644
--- a/sysdeps/unix/bsd/wait3.c
+++ b/sysdeps/unix/bsd/wait3.c
@@ -25,7 +25,7 @@
    there.  If the WUNTRACED bit is set in OPTIONS, return status for stopped
    children; otherwise don't.  */
 pid_t
-__wait3 (__WAIT_STATUS stat_loc, int options, struct rusage *usage)
+__wait3 (int *stat_loc, int options, struct rusage *usage)
 {
   return __wait4 (WAIT_ANY, stat_loc, options, usage);
 }
diff --git a/sysdeps/unix/bsd/waitpid.c b/sysdeps/unix/bsd/waitpid.c
index 083c686..cfe5614 100644
--- a/sysdeps/unix/bsd/waitpid.c
+++ b/sysdeps/unix/bsd/waitpid.c
@@ -35,7 +35,7 @@
 pid_t
 __waitpid (pid_t pid, int *stat_loc, int options)
 {
-  return __wait4 (pid, (union wait *) stat_loc, options, NULL);
+  return __wait4 (pid, stat_loc, options, NULL);
 }
 
 libc_hidden_def (__waitpid)
diff --git a/sysdeps/unix/sysv/linux/wait.c b/sysdeps/unix/sysv/linux/wait.c
index 71f9044..9b7c9c9 100644
--- a/sysdeps/unix/sysv/linux/wait.c
+++ b/sysdeps/unix/sysv/linux/wait.c
@@ -24,7 +24,7 @@
 /* Wait for a child to die.  When one does, put its status in *STAT_LOC
    and return its process ID.  For errors, return (pid_t) -1.  */
 pid_t
-__libc_wait (__WAIT_STATUS_DEFN stat_loc)
+__libc_wait (int *stat_loc)
 {
   pid_t result = SYSCALL_CANCEL (wait4, WAIT_ANY, stat_loc, 0,
 				 (struct rusage *) NULL);

Re: [PATCH v2] Remove union wait

[Florian Weimer]

On 03/08/2016 10:57 AM, Florian Weimer wrote:
> 
> 2016-03-08  Florian Weimer  <fweimer@redhat.com>
> 
> 	[BZ #19613]
> 	Remove union wait.
> 	* bits/waitstatus.h (union wait, w_termsig, w_coredump, w_retcode)
> 	(w_stopsig, w_stopval): Remove.
> 	* include/sys/wait.h (__wait, __wait3, __wait4): Use int * for the
> 	stat_loc argument.
> 	* posix/sys/wait.h (__WAIT_INT, __WAIT_STATUS)
> 	(__WAIT_STATUS_DEFN): Remove.
> 	(WEXITSTATUS, WTERMSIG, WSTOPSIG, WIFEXITED, WIFSIGNALED)
> 	(WIFSTOPPED, WIFCONTINUED, WCOREDUMP): Remove __WAIT_INT.
> 	(wait, wait3, wait4): Use int * for the stat_loc argument.
> 	* posix/wait.c (__wait): Likewise.
> 	* posix/wait3.c (__wait3): Likewise.
> 	* posix/wait4.c (__wait4): Likewise.
> 	* stdlib/stdlib.h (__WAIT_INT, __WAIT_STATUS)
> 	(__WAIT_STATUS_DEFN): Remove.
> 	(WEXITSTATUS, WTERMSIG, WSTOPSIG, WIFEXITED, WIFSIGNALED)
> 	(WIFSTOPPED, WIFCONTINUED): Remove __WAIT_INT.
> 	* sysdeps/mach/hurd/wait4.c (__wait4): Use int * for the stat_loc
> 	argument.
> 	* sysdeps/posix/wait.c (__libc_wait): Likewise.
> 	* sysdeps/posix/wait3.c (__wait3): Likewise.
> 	* sysdeps/unix/bsd/wait.c (__libc_wait): Likewise.
> 	* sysdeps/unix/bsd/wait3.c (__wait3): Likewise.
> 	* sysdeps/unix/bsd/waitpid.c (__waitpid): Remove cast.
> 	* sysdeps/unix/sysv/linux/wait.c (__libc_wait): Use int * for the
> 	stat_loc argument.
> 	* manual/process.texi (BSD Wait Functions): Remove union wait.

Ping?

  https://sourceware.org/ml/libc-alpha/2016-03/msg00200.html

Florian

Re: [PATCH v2] Remove union wait

[Roland McGrath]

I think the blocker here is clear consensus that a one-cycle removal of a
feature from the API without any broadcasted deprecation period is OK.
I'd like some other senior maintainers to explicitly agree with that before
we go ahead based just on the direct review of the change itself.


Thanks,
Roland

Re: [PATCH v2] Remove union wait (was: Re: Deprecate union wait and remove support from wait functions [BZ #19613])

[Roland McGrath]

All the code substance of the change looks fine to me (separate from the
high-level issue of whether to do this change now).

> --- a/NEWS
> +++ b/NEWS
> @@ -21,6 +21,10 @@ Version 2.24
>  * The readdir_r and readdir64_r functions have been deprecated.  It is
>    recommended to use readdir and readdir64 instead.
>  
> +* The type union wait has been removed.  

I would put some punctuation of some sort around the two-word type name.
Perhaps 'union wait'.

> It was obsoleted by 4.3+BSD in the
> +  early 1990s.  

This is inaccurate.  'union wait *' was the sole type for wait and wait3 in
4.3BSD.  As of 4.4-BSD-Lite2 (1995), the functions' prototypes used 'int *'
(so there would be a compiler warning passing 'union wait *' to them, but
it would still work), the 'union wait' type was still present, and the W*
macros worked with either (by virtue of a sloppy cast in the macros).

Looking at https://svnweb.freebsd.org/base/head/sys/sys/wait.h AFAICT
FreeBSD's trunk version dropped 'union wait' entirely in 2002.  I'm not
sure what the first FreeBSD release version was that lacked it.

> Application code should use the int type instead of union
> +  wait.

Perhaps it's simpler just to say that it's been obsolete ever since the
standardization of POSIX.1 and rarely used "for 20 years or more" or
something like that.

> diff --git a/manual/process.texi b/manual/process.texi
> index aa59040..25bdb8e 100644
> --- a/manual/process.texi
> +++ b/manual/process.texi
> @@ -763,53 +763,17 @@ signal number of the signal that caused the child process to stop.
>  
>  
>  @node BSD Wait Functions
> -@section BSD Process Wait Functions
> +@section BSD Process Wait Function

This should probably be more thoroughly rewritten, probably dropping the
section and describing wait3 as obsolete just alongside wait4.


Thanks,
Roland

Re: [PATCH v2] Remove union wait

[Carlos O'Donell]

On 03/30/2016 06:32 PM, Roland McGrath wrote:
> I think the blocker here is clear consensus that a one-cycle removal of a
> feature from the API without any broadcasted deprecation period is OK.
> I'd like some other senior maintainers to explicitly agree with that before
> we go ahead based just on the direct review of the change itself.

IMO a one-cycle removal is OK IFF a distribution has gone through a full build
cycle with the deprecation patch applied and found no serious problems in the
existing corpus of package sources.

An example would be if Fedora Rawhide ran 4-6 months with the patch and nothing
fell out of the rebuild cycle.

Given that we have not done done a build cycle in a distribution I think the
most conservative step is a staged removal. I think your original instinct is
the best way forward e.g. removal in 2.25.

Please remember to write notes under 2.24's packaging changes:
https://sourceware.org/glibc/wiki/Release/2.24#Packaging_Changes

Which detail the changes that packages need to make to migrate away from the
old interface to the new interface.

-- 
Cheers,
Carlos.

Re: [PATCH v2] Remove union wait

[Andreas Schwab]

Carlos O'Donell <carlos@redhat.com> writes:

> On 03/30/2016 06:32 PM, Roland McGrath wrote:
>> I think the blocker here is clear consensus that a one-cycle removal of a
>> feature from the API without any broadcasted deprecation period is OK.
>> I'd like some other senior maintainers to explicitly agree with that before
>> we go ahead based just on the direct review of the change itself.
>
> IMO a one-cycle removal is OK IFF a distribution has gone through a full build
> cycle with the deprecation patch applied and found no serious problems in the
> existing corpus of package sources.

I've now set up
<https://build.opensuse.org/project/show/home:Andreas_Schwab:glibc> to
rebuild all packages from
<https://build.opensuse.org/project/show/openSUSE:Factory:Rings:1-MinimalX>
against a patched glibc.  The latter project contains the packages that
we use for testing all updates for our openSUSE Tumbleweed distribution.
You can monitor progress at
<https://build.opensuse.org/project/monitor/home:Andreas_Schwab:glibc>.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH v2] Remove union wait

[Florian Weimer]

On 03/31/2016 09:14 AM, Andreas Schwab wrote:
> Carlos O'Donell <carlos@redhat.com> writes:
> 
>> On 03/30/2016 06:32 PM, Roland McGrath wrote:
>>> I think the blocker here is clear consensus that a one-cycle removal of a
>>> feature from the API without any broadcasted deprecation period is OK.
>>> I'd like some other senior maintainers to explicitly agree with that before
>>> we go ahead based just on the direct review of the change itself.
>>
>> IMO a one-cycle removal is OK IFF a distribution has gone through a full build
>> cycle with the deprecation patch applied and found no serious problems in the
>> existing corpus of package sources.
> 
> I've now set up
> <https://build.opensuse.org/project/show/home:Andreas_Schwab:glibc> to
> rebuild all packages from
> <https://build.opensuse.org/project/show/openSUSE:Factory:Rings:1-MinimalX>
> against a patched glibc.  The latter project contains the packages that
> we use for testing all updates for our openSUSE Tumbleweed distribution.
> You can monitor progress at
> <https://build.opensuse.org/project/monitor/home:Andreas_Schwab:glibc>.

Nice piece of technology. :)  Thanks for doing this.

Somehow, the earlier tcsh build failure I saw on the monitor page is
gone.  tcsh has a __GLIBC__ feature check, and should avoid using union
wait there, but the macro is probably checked without including
<features.h>, so __GLIBC__ is not defined.

The other build failures where due to the readdir_r deprecation.

Florian

Re: [PATCH v2] Remove union wait

[Andreas Schwab]

Florian Weimer <fweimer@redhat.com> writes:

> Somehow, the earlier tcsh build failure I saw on the monitor page is
> gone.

It's still there, but all packages are currently scheduled for rebuild
due to updates to openSUSE:Factory.  You can tick "Last time results" to
see the last finished build results.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH v2] Remove union wait

[Roland McGrath]

> Given that we have not done done a build cycle in a distribution I think the
> most conservative step is a staged removal. I think your original instinct is
> the best way forward e.g. removal in 2.25.

Do you have a precise definition of "staged removal" in mind?

My first guess at what this means is that now (for 2.24) we add
__attribute_deprecated__ to the 'union wait' type definition, and then
shortly after 2.24 branches we remove it entirely.  I'm not sure about
documentation changes.


Thanks,
Roland

Re: [PATCH v2] Remove union wait

[Florian Weimer]

On 03/31/2016 12:47 AM, Roland McGrath wrote:

>> It was obsoleted by 4.3+BSD in the
>> +  early 1990s.  
> 
> This is inaccurate.  'union wait *' was the sole type for wait and wait3 in
> 4.3BSD.

Oh.  I wasn't aware that 4.3+BSD was a term made up by Richard Stevens
for the 1993 edition of Advanced Programming in the UNIX Environment
(which predates 4.4BSD).

> As of 4.4-BSD-Lite2 (1995), the functions' prototypes used 'int *'
> (so there would be a compiler warning passing 'union wait *' to them, but
> it would still work), the 'union wait' type was still present, and the W*
> macros worked with either (by virtue of a sloppy cast in the macros).

In any case, union wait is not documented at all in the book.

> Perhaps it's simpler just to say that it's been obsolete ever since the
> standardization of POSIX.1 and rarely used "for 20 years or more" or
> something like that.

There is the incorrect feature macro check in tcsh (yet another
instance, they accidentally replaced the system malloc due to the same
mistake), as identified by Andreas' rebuild.  It's the only failure I
saw among the results, though.

>>  @node BSD Wait Functions
>> -@section BSD Process Wait Functions
>> +@section BSD Process Wait Function
> 
> This should probably be more thoroughly rewritten, probably dropping the
> section and describing wait3 as obsolete just alongside wait4.

That's a separate patch, right?

Florian

Re: [PATCH v2] Remove union wait

[Roland McGrath]

> Oh.  I wasn't aware that 4.3+BSD was a term made up by Richard Stevens
> for the 1993 edition of Advanced Programming in the UNIX Environment
> (which predates 4.4BSD).

The last book I read about Unix programming was the first edition of K&R
(unless you count the hardcopy set of of 4.3BSD man pages I had).  I only
refer to the sources, header files, and machine-readable documentation
that's part of the system.

> In any case, union wait is not documented at all in the book.

An example of why such books are not good resources for authoritative
information.

> > This should probably be more thoroughly rewritten, probably dropping the
> > section and describing wait3 as obsolete just alongside wait4.
> 
> That's a separate patch, right?

Either way, as long as it doesn't get forgotten and doesn't lag long past
the code patch.  The state your patch leaves the manual in looks a bit like
the state source code gets in after a patch that removes a bunch of code
but doesn't remove comments that refer to that code.

Re: [PATCH v2] Remove union wait

[Carlos O'Donell]

On 04/01/2016 05:29 PM, Roland McGrath wrote:
>> Given that we have not done done a build cycle in a distribution I think the
>> most conservative step is a staged removal. I think your original instinct is
>> the best way forward e.g. removal in 2.25.
> 
> Do you have a precise definition of "staged removal" in mind?
> 
> My first guess at what this means is that now (for 2.24) we add
> __attribute_deprecated__ to the 'union wait' type definition, and then
> shortly after 2.24 branches we remove it entirely.  I'm not sure about
> documentation changes.

Exactly that. Deprecate in one release. Remove in the next.

For documentation you would need to make sure the manual is updated if
it contains any references, and update the Release/2.24 and Release/2.25
wiki pages to mention the deprecation and removal.

-- 
Cheers,
Carlos.

Re: [PATCH v2] Remove union wait

[Andreas Schwab]

Florian Weimer <fweimer@redhat.com> writes:

> Somehow, the earlier tcsh build failure I saw on the monitor page is
> gone.  tcsh has a __GLIBC__ feature check, and should avoid using union
> wait there, but the macro is probably checked without including
> <features.h>, so __GLIBC__ is not defined.

I think you got it backwards.  When __GLIBC__ is defined, tcsh
explicitly defines BSDWAIT, which means to use union wait.  Handling
__GLIBC__ like __ANDROID__ will fix that.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH v2] Remove union wait

[Florian Weimer]

On 03/31/2016 09:14 AM, Andreas Schwab wrote:
> Carlos O'Donell <carlos@redhat.com> writes:
> 
>> On 03/30/2016 06:32 PM, Roland McGrath wrote:
>>> I think the blocker here is clear consensus that a one-cycle removal of a
>>> feature from the API without any broadcasted deprecation period is OK.
>>> I'd like some other senior maintainers to explicitly agree with that before
>>> we go ahead based just on the direct review of the change itself.
>>
>> IMO a one-cycle removal is OK IFF a distribution has gone through a full build
>> cycle with the deprecation patch applied and found no serious problems in the
>> existing corpus of package sources.
> 
> I've now set up
> <https://build.opensuse.org/project/show/home:Andreas_Schwab:glibc> to
> rebuild all packages from
> <https://build.opensuse.org/project/show/openSUSE:Factory:Rings:1-MinimalX>
> against a patched glibc.  The latter project contains the packages that
> we use for testing all updates for our openSUSE Tumbleweed distribution.
> You can monitor progress at
> <https://build.opensuse.org/project/monitor/home:Andreas_Schwab:glibc>.

I looked at the output.  If I read it correctly, the situation is like this:

The only build failure attributable to the “union wait” removal is tcsh.
 It just needs fixing.

For readdir_r, we were opting for a staged removal (flag it as
deprecated, then remove it after it is gone from POSIX).  But the
deprecation warning itself causes build failures.  This is the majority
of the failed builds Andreas' build service lists.

To me, this suggests to proceed with the “union wait” removal.

Florian

Re: [PATCH v2] Remove union wait

[Andreas Schwab]

Florian Weimer <fweimer@redhat.com> writes:

> The only build failure attributable to the “union wait” removal is tcsh.
>  It just needs fixing.

I have already submitted a fix for tcsh.

> To me, this suggests to proceed with the “union wait” removal.

I aggree.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH v2] Remove union wait

[Florian Weimer]

[-- Attachment #1: Type: text/plain, Size: 624 bytes --]

On 04/11/2016 11:06 AM, Andreas Schwab wrote:
> Florian Weimer <fweimer@redhat.com> writes:
>
>> The only build failure attributable to the “union wait” removal is tcsh.
>>   It just needs fixing.
>
> I have already submitted a fix for tcsh.

Thanks. :)

>> To me, this suggests to proceed with the “union wait” removal.
>
> I aggree.

What about this patch?

I massaged the NEWS entry a bit, as suggested by Roland.  I did not 
include the additional manual change because I noticed that wait3 has 
not been formally deprecated in the header file, so I want to keep that 
in a separate patch.

Florian


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Remove-union-wait-BZ-19613.patch --]
[-- Type: text/x-patch; name="0001-Remove-union-wait-BZ-19613.patch", Size: 16796 bytes --]

2016-04-11  Florian Weimer  <fweimer@redhat.com>

	[BZ #19613]
	Remove union wait.
	* bits/waitstatus.h (union wait, w_termsig, w_coredump, w_retcode)
	(w_stopsig, w_stopval): Remove.
	* include/sys/wait.h (__wait, __wait3, __wait4): Use int * for the
	stat_loc argument.
	* posix/sys/wait.h (__WAIT_INT, __WAIT_STATUS)
	(__WAIT_STATUS_DEFN): Remove.
	(WEXITSTATUS, WTERMSIG, WSTOPSIG, WIFEXITED, WIFSIGNALED)
	(WIFSTOPPED, WIFCONTINUED, WCOREDUMP): Remove __WAIT_INT.
	(wait, wait3, wait4): Use int * for the stat_loc argument.
	* posix/wait.c (__wait): Likewise.
	* posix/wait3.c (__wait3): Likewise.
	* posix/wait4.c (__wait4): Likewise.
	* stdlib/stdlib.h (__WAIT_INT, __WAIT_STATUS)
	(__WAIT_STATUS_DEFN): Remove.
	(WEXITSTATUS, WTERMSIG, WSTOPSIG, WIFEXITED, WIFSIGNALED)
	(WIFSTOPPED, WIFCONTINUED): Remove __WAIT_INT.
	* sysdeps/mach/hurd/wait4.c (__wait4): Use int * for the stat_loc
	argument.
	* sysdeps/posix/wait.c (__libc_wait): Likewise.
	* sysdeps/posix/wait3.c (__wait3): Likewise.
	* sysdeps/unix/bsd/wait.c (__libc_wait): Likewise.
	* sysdeps/unix/bsd/wait3.c (__wait3): Likewise.
	* sysdeps/unix/bsd/waitpid.c (__waitpid): Remove cast.
	* sysdeps/unix/sysv/linux/wait.c (__libc_wait): Use int * for the
	stat_loc argument.
	* manual/process.texi (BSD Wait Functions): Remove union wait.

diff --git a/NEWS b/NEWS
index a06a42f..54ffb02 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,10 @@ Version 2.24
 * The readdir_r and readdir64_r functions have been deprecated.  It is
   recommended to use readdir and readdir64 instead.
 
+* The type “union wait” has been removed.  It was deprecated in the early
+  1990s and never part of POSIX.  Application code should use the int type
+  instead of “union wait”.
+
 Security related changes:
 
   [Add security related changes here]
diff --git a/bits/waitstatus.h b/bits/waitstatus.h
index 38c33bc..069ce4b 100644
--- a/bits/waitstatus.h
+++ b/bits/waitstatus.h
@@ -57,49 +57,3 @@
 #define	__W_STOPCODE(sig)	((sig) << 8 | 0x7f)
 #define __W_CONTINUED		0xffff
 #define	__WCOREFLAG		0x80
-
-
-#ifdef	__USE_MISC
-
-# include <endian.h>
-
-union wait
-  {
-    int w_status;
-    struct
-      {
-# if	__BYTE_ORDER == __LITTLE_ENDIAN
-	unsigned int __w_termsig:7; /* Terminating signal.  */
-	unsigned int __w_coredump:1; /* Set if dumped core.  */
-	unsigned int __w_retcode:8; /* Return code if exited normally.  */
-	unsigned int:16;
-# endif				/* Little endian.  */
-# if	__BYTE_ORDER == __BIG_ENDIAN
-	unsigned int:16;
-	unsigned int __w_retcode:8;
-	unsigned int __w_coredump:1;
-	unsigned int __w_termsig:7;
-# endif				/* Big endian.  */
-      } __wait_terminated;
-    struct
-      {
-# if	__BYTE_ORDER == __LITTLE_ENDIAN
-	unsigned int __w_stopval:8; /* W_STOPPED if stopped.  */
-	unsigned int __w_stopsig:8; /* Stopping signal.  */
-	unsigned int:16;
-# endif				/* Little endian.  */
-# if	__BYTE_ORDER == __BIG_ENDIAN
-	unsigned int:16;
-	unsigned int __w_stopsig:8; /* Stopping signal.  */
-	unsigned int __w_stopval:8; /* W_STOPPED if stopped.  */
-# endif				/* Big endian.  */
-      } __wait_stopped;
-  };
-
-# define w_termsig	__wait_terminated.__w_termsig
-# define w_coredump	__wait_terminated.__w_coredump
-# define w_retcode	__wait_terminated.__w_retcode
-# define w_stopsig	__wait_stopped.__w_stopsig
-# define w_stopval	__wait_stopped.__w_stopval
-
-#endif	/* Use misc.  */
diff --git a/include/sys/wait.h b/include/sys/wait.h
index 9a38e61..5ac9cd6 100644
--- a/include/sys/wait.h
+++ b/include/sys/wait.h
@@ -9,10 +9,10 @@ libc_hidden_proto (__waitpid)
 extern int __waitid (idtype_t idtype, id_t id, siginfo_t *infop, int options);
 
 extern __pid_t __libc_wait (int *__stat_loc);
-extern __pid_t __wait (__WAIT_STATUS __stat_loc);
-extern __pid_t __wait3 (__WAIT_STATUS __stat_loc,
+extern __pid_t __wait (int *__stat_loc);
+extern __pid_t __wait3 (int *__stat_loc,
 			int __options, struct rusage * __usage);
-extern __pid_t __wait4 (__pid_t __pid, __WAIT_STATUS __stat_loc,
+extern __pid_t __wait4 (__pid_t __pid, int *__stat_loc,
 			int __options, struct rusage *__usage)
 			attribute_hidden;
 #endif
diff --git a/manual/process.texi b/manual/process.texi
index aa59040..25bdb8e 100644
--- a/manual/process.texi
+++ b/manual/process.texi
@@ -763,53 +763,17 @@ signal number of the signal that caused the child process to stop.
 
 
 @node BSD Wait Functions
-@section BSD Process Wait Functions
+@section BSD Process Wait Function
 
-@Theglibc{} also provides these related facilities for compatibility
-with BSD Unix.  BSD uses the @code{union wait} data type to represent
-status values rather than an @code{int}.  The two representations are
-actually interchangeable; they describe the same bit patterns.  @Theglibc{}
-defines macros such as @code{WEXITSTATUS} so that they will
-work on either kind of object, and the @code{wait} function is defined
-to accept either type of pointer as its @var{status-ptr} argument.
-
-These functions are declared in @file{sys/wait.h}.
+@Theglibc{} also provides the @code{wait3} function for compatibility
+with BSD.  This function is declared in @file{sys/wait.h}.  It is the
+predecessor to @code{wait4}, which is more flexible.  @code{wait3} is
+now obsolete.
 @pindex sys/wait.h
 
 @comment sys/wait.h
 @comment BSD
-@deftp {Data Type} {union wait}
-This data type represents program termination status values.  It has
-the following members:
-
-@table @code
-@item int w_termsig
-The value of this member is the same as that of the
-@code{WTERMSIG} macro.
-
-@item int w_coredump
-The value of this member is the same as that of the
-@code{WCOREDUMP} macro.
-
-@item int w_retcode
-The value of this member is the same as that of the
-@code{WEXITSTATUS} macro.
-
-@item int w_stopsig
-The value of this member is the same as that of the
-@code{WSTOPSIG} macro.
-@end table
-
-Instead of accessing these members directly, you should use the
-equivalent macros.
-@end deftp
-
-The @code{wait3} function is the predecessor to @code{wait4}, which is
-more flexible.  @code{wait3} is now obsolete.
-
-@comment sys/wait.h
-@comment BSD
-@deftypefun pid_t wait3 (union wait *@var{status-ptr}, int @var{options}, struct rusage *@var{usage})
+@deftypefun pid_t wait3 (int *@var{status-ptr}, int @var{options}, struct rusage *@var{usage})
 @safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
 If @var{usage} is a null pointer, @code{wait3} is equivalent to
 @code{waitpid (-1, @var{status-ptr}, @var{options})}.
diff --git a/posix/sys/wait.h b/posix/sys/wait.h
index a673b41..acda43a 100644
--- a/posix/sys/wait.h
+++ b/posix/sys/wait.h
@@ -34,62 +34,23 @@ __BEGIN_DECLS
    bits to `waitpid', `wait3', and `wait4'.  */
 # include <bits/waitflags.h>
 
-# ifdef	__USE_MISC
-
-/* Lots of hair to allow traditional BSD use of `union wait'
-   as well as POSIX.1 use of `int' for the status word.  */
-
-#  if defined __GNUC__ && !defined __cplusplus
-#   define __WAIT_INT(status) \
-  (__extension__ (((union { __typeof(status) __in; int __i; }) \
-		   { .__in = (status) }).__i))
-#  else
-#   define __WAIT_INT(status)	(*(const int *) &(status))
-#  endif
-
-/* This is the type of the argument to `wait'.  The funky union
-   causes redeclarations with either `int *' or `union wait *' to be
-   allowed without complaint.  __WAIT_STATUS_DEFN is the type used in
-   the actual function definitions.  */
-
-#  if !defined __GNUC__ || __GNUC__ < 2 || defined __cplusplus
-#   define __WAIT_STATUS	void *
-#   define __WAIT_STATUS_DEFN	void *
-#  else
-/* This works in GCC 2.6.1 and later.  */
-typedef union
-  {
-    union wait *__uptr;
-    int *__iptr;
-  } __WAIT_STATUS __attribute__ ((__transparent_union__));
-#   define __WAIT_STATUS_DEFN	int *
-#  endif
-
-# else /* Don't use misc.  */
-
-#  define __WAIT_INT(status)	(status)
-#  define __WAIT_STATUS		int *
-#  define __WAIT_STATUS_DEFN	int *
-
-# endif /* Use misc.  */
-
 /* This will define all the `__W*' macros.  */
 # include <bits/waitstatus.h>
 
-# define WEXITSTATUS(status)	__WEXITSTATUS (__WAIT_INT (status))
-# define WTERMSIG(status)	__WTERMSIG (__WAIT_INT (status))
-# define WSTOPSIG(status)	__WSTOPSIG (__WAIT_INT (status))
-# define WIFEXITED(status)	__WIFEXITED (__WAIT_INT (status))
-# define WIFSIGNALED(status)	__WIFSIGNALED (__WAIT_INT (status))
-# define WIFSTOPPED(status)	__WIFSTOPPED (__WAIT_INT (status))
+# define WEXITSTATUS(status)	__WEXITSTATUS (status)
+# define WTERMSIG(status)	__WTERMSIG (status)
+# define WSTOPSIG(status)	__WSTOPSIG (status)
+# define WIFEXITED(status)	__WIFEXITED (status)
+# define WIFSIGNALED(status)	__WIFSIGNALED (status)
+# define WIFSTOPPED(status)	__WIFSTOPPED (status)
 # ifdef __WIFCONTINUED
-#  define WIFCONTINUED(status)	__WIFCONTINUED (__WAIT_INT (status))
+#  define WIFCONTINUED(status)	__WIFCONTINUED (status)
 # endif
 #endif	/* <stdlib.h> not included.  */
 
 #ifdef	__USE_MISC
 # define WCOREFLAG		__WCOREFLAG
-# define WCOREDUMP(status)	__WCOREDUMP (__WAIT_INT (status))
+# define WCOREDUMP(status)	__WCOREDUMP (status)
 # define W_EXITCODE(ret, sig)	__W_EXITCODE (ret, sig)
 # define W_STOPCODE(sig)	__W_STOPCODE (sig)
 #endif
@@ -110,7 +71,7 @@ typedef enum
 
    This function is a cancellation point and therefore not marked with
    __THROW.  */
-extern __pid_t wait (__WAIT_STATUS __stat_loc);
+extern __pid_t wait (int *__stat_loc);
 
 #ifdef	__USE_MISC
 /* Special values for the PID argument to `waitpid' and `wait4'.  */
@@ -170,13 +131,13 @@ struct rusage;
    nil, store information about the child's resource usage there.  If the
    WUNTRACED bit is set in OPTIONS, return status for stopped children;
    otherwise don't.  */
-extern __pid_t wait3 (__WAIT_STATUS __stat_loc, int __options,
+extern __pid_t wait3 (int *__stat_loc, int __options,
 		      struct rusage * __usage) __THROWNL;
 #endif
 
 #ifdef __USE_MISC
 /* PID is like waitpid.  Other args are like wait3.  */
-extern __pid_t wait4 (__pid_t __pid, __WAIT_STATUS __stat_loc, int __options,
+extern __pid_t wait4 (__pid_t __pid, int *__stat_loc, int __options,
 		      struct rusage *__usage) __THROWNL;
 #endif /* Use misc.  */
 
diff --git a/posix/wait.c b/posix/wait.c
index 3abf30e..c49375b 100644
--- a/posix/wait.c
+++ b/posix/wait.c
@@ -21,7 +21,7 @@
 /* Wait for a child to die.  When one does, put its status in *STAT_LOC
    and return its process ID.  For errors, return (pid_t) -1.  */
 __pid_t
-__wait (__WAIT_STATUS_DEFN stat_loc)
+__wait (int *stat_loc)
 {
   __set_errno (ENOSYS);
   return -1;
diff --git a/posix/wait3.c b/posix/wait3.c
index 356839a..e8c2930 100644
--- a/posix/wait3.c
+++ b/posix/wait3.c
@@ -25,7 +25,7 @@
    there.  If the WUNTRACED bit is set in OPTIONS, return status for stopped
    children; otherwise don't.  */
 pid_t
-__wait3 (__WAIT_STATUS_DEFN stat_loc, int options, struct rusage *usage)
+__wait3 (int *stat_loc, int options, struct rusage *usage)
 {
   if ((options & ~(WNOHANG|WUNTRACED)) != 0)
     {
diff --git a/posix/wait4.c b/posix/wait4.c
index e5b0376..4137617 100644
--- a/posix/wait4.c
+++ b/posix/wait4.c
@@ -20,8 +20,7 @@
 #include <errno.h>
 
 pid_t
-__wait4 (__pid_t pid, __WAIT_STATUS stat_loc, int options,
-	 struct rusage *usage)
+__wait4 (__pid_t pid, int *stat_loc, int options, struct rusage *usage)
 {
   __set_errno (ENOSYS);
   return (pid_t) -1;
diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
index cc77708..d0c78fa 100644
--- a/stdlib/stdlib.h
+++ b/stdlib/stdlib.h
@@ -41,54 +41,15 @@ __BEGIN_DECLS
 # include <bits/waitflags.h>
 # include <bits/waitstatus.h>
 
-# ifdef __USE_MISC
-
-/* Lots of hair to allow traditional BSD use of `union wait'
-   as well as POSIX.1 use of `int' for the status word.  */
-
-#  if defined __GNUC__ && !defined __cplusplus
-#   define __WAIT_INT(status) \
-  (__extension__ (((union { __typeof(status) __in; int __i; }) \
-		   { .__in = (status) }).__i))
-#  else
-#   define __WAIT_INT(status)	(*(int *) &(status))
-#  endif
-
-/* This is the type of the argument to `wait'.  The funky union
-   causes redeclarations with either `int *' or `union wait *' to be
-   allowed without complaint.  __WAIT_STATUS_DEFN is the type used in
-   the actual function definitions.  */
-
-#  if !defined __GNUC__ || __GNUC__ < 2 || defined __cplusplus
-#   define __WAIT_STATUS	void *
-#   define __WAIT_STATUS_DEFN	void *
-#  else
-/* This works in GCC 2.6.1 and later.  */
-typedef union
-  {
-    union wait *__uptr;
-    int *__iptr;
-  } __WAIT_STATUS __attribute__ ((__transparent_union__));
-#   define __WAIT_STATUS_DEFN	int *
-#  endif
-
-# else /* Don't use misc.  */
-
-#  define __WAIT_INT(status)	(status)
-#  define __WAIT_STATUS		int *
-#  define __WAIT_STATUS_DEFN	int *
-
-# endif /* Use misc.  */
-
 /* Define the macros <sys/wait.h> also would define this way.  */
-# define WEXITSTATUS(status)	__WEXITSTATUS (__WAIT_INT (status))
-# define WTERMSIG(status)	__WTERMSIG (__WAIT_INT (status))
-# define WSTOPSIG(status)	__WSTOPSIG (__WAIT_INT (status))
-# define WIFEXITED(status)	__WIFEXITED (__WAIT_INT (status))
-# define WIFSIGNALED(status)	__WIFSIGNALED (__WAIT_INT (status))
-# define WIFSTOPPED(status)	__WIFSTOPPED (__WAIT_INT (status))
+# define WEXITSTATUS(status)	__WEXITSTATUS (status)
+# define WTERMSIG(status)	__WTERMSIG (status)
+# define WSTOPSIG(status)	__WSTOPSIG (status)
+# define WIFEXITED(status)	__WIFEXITED (status)
+# define WIFSIGNALED(status)	__WIFSIGNALED (status)
+# define WIFSTOPPED(status)	__WIFSTOPPED (status)
 # ifdef __WIFCONTINUED
-#  define WIFCONTINUED(status)	__WIFCONTINUED (__WAIT_INT (status))
+#  define WIFCONTINUED(status)	__WIFCONTINUED (status)
 # endif
 #endif	/* X/Open or XPG7 and <sys/wait.h> not included.  */
 
diff --git a/sysdeps/mach/hurd/wait4.c b/sysdeps/mach/hurd/wait4.c
index 3bc9fa8..f392a98 100644
--- a/sysdeps/mach/hurd/wait4.c
+++ b/sysdeps/mach/hurd/wait4.c
@@ -22,8 +22,7 @@
 #include <hurd/port.h>
 
 pid_t
-__wait4 (pid_t pid, __WAIT_STATUS_DEFN stat_loc, int options,
-	 struct rusage *usage)
+__wait4 (pid_t pid, int *stat_loc, int options, struct rusage *usage)
 {
   pid_t dead;
   error_t err;
diff --git a/sysdeps/posix/wait.c b/sysdeps/posix/wait.c
index 7f1d71a..210ece8 100644
--- a/sysdeps/posix/wait.c
+++ b/sysdeps/posix/wait.c
@@ -21,7 +21,7 @@
 /* Wait for a child to die.  When one does, put its status in *STAT_LOC
    and return its process ID.  For errors, return (pid_t) -1.  */
 __pid_t
-__libc_wait (__WAIT_STATUS_DEFN stat_loc)
+__libc_wait (int *stat_loc)
 {
   return __waitpid (WAIT_ANY, (int *) stat_loc, 0);
 }
diff --git a/sysdeps/posix/wait3.c b/sysdeps/posix/wait3.c
index 2e76892..cf43d97 100644
--- a/sysdeps/posix/wait3.c
+++ b/sysdeps/posix/wait3.c
@@ -26,7 +26,7 @@
    there.  If the WUNTRACED bit is set in OPTIONS, return status for stopped
    children; otherwise don't.  */
 pid_t
-__wait3 (__WAIT_STATUS stat_loc, int options, struct rusage *usage)
+__wait3 (int *stat_loc, int options, struct rusage *usage)
 {
   if (usage != NULL)
     {
diff --git a/sysdeps/unix/bsd/wait.c b/sysdeps/unix/bsd/wait.c
index 31de60e..a9e29f2 100644
--- a/sysdeps/unix/bsd/wait.c
+++ b/sysdeps/unix/bsd/wait.c
@@ -23,7 +23,7 @@
 /* Wait for a child to die.  When one does, put its status in *STAT_LOC
    and return its process ID.  For errors, return (pid_t) -1.  */
 __pid_t
-__libc_wait (__WAIT_STATUS_DEFN stat_loc)
+__libc_wait (int *stat_loc)
 {
   return __wait4 (WAIT_ANY, stat_loc, 0, (struct rusage *) NULL);
 }
diff --git a/sysdeps/unix/bsd/wait3.c b/sysdeps/unix/bsd/wait3.c
index 3f95ac7..ee33a65 100644
--- a/sysdeps/unix/bsd/wait3.c
+++ b/sysdeps/unix/bsd/wait3.c
@@ -25,7 +25,7 @@
    there.  If the WUNTRACED bit is set in OPTIONS, return status for stopped
    children; otherwise don't.  */
 pid_t
-__wait3 (__WAIT_STATUS stat_loc, int options, struct rusage *usage)
+__wait3 (int *stat_loc, int options, struct rusage *usage)
 {
   return __wait4 (WAIT_ANY, stat_loc, options, usage);
 }
diff --git a/sysdeps/unix/bsd/waitpid.c b/sysdeps/unix/bsd/waitpid.c
index 083c686..cfe5614 100644
--- a/sysdeps/unix/bsd/waitpid.c
+++ b/sysdeps/unix/bsd/waitpid.c
@@ -35,7 +35,7 @@
 pid_t
 __waitpid (pid_t pid, int *stat_loc, int options)
 {
-  return __wait4 (pid, (union wait *) stat_loc, options, NULL);
+  return __wait4 (pid, stat_loc, options, NULL);
 }
 
 libc_hidden_def (__waitpid)
diff --git a/sysdeps/unix/sysv/linux/wait.c b/sysdeps/unix/sysv/linux/wait.c
index 71f9044..9b7c9c9 100644
--- a/sysdeps/unix/sysv/linux/wait.c
+++ b/sysdeps/unix/sysv/linux/wait.c
@@ -24,7 +24,7 @@
 /* Wait for a child to die.  When one does, put its status in *STAT_LOC
    and return its process ID.  For errors, return (pid_t) -1.  */
 pid_t
-__libc_wait (__WAIT_STATUS_DEFN stat_loc)
+__libc_wait (int *stat_loc)
 {
   pid_t result = SYSCALL_CANCEL (wait4, WAIT_ANY, stat_loc, 0,
 				 (struct rusage *) NULL);

Re: [PATCH v2] Remove union wait

[Roland McGrath]

The substance of the change looks OK to me.  (I spent a little while
deciding how much I object to the non-ASCII double-quotes you used in
NEWS.  But since there are some non-ASCII characters elsewhere in the
file so I'll restrain my Amerocentric Luddism for now.)

I still don't have much opinion either way about staged deprecation.
Since few others have opined, I think the decision now is up to
whether Carlos changes his mind.  

But I will point out that the claim that the deprecation warnings are
just as bad as outright removal is not really right.  Packagers or
system-builders or whoever can add -Wno-error=deprecated to the
compilation flags for a particular package via configure, or for an
entire package-building system in that system's configuration for
building all packages (a la redhat-rpm-config), etc.  Those are all
things that can be done without patching any sources.  When you remove
the feature altogether, there is no way to adjust the build process so
that it continues to work without source code changes.


Thanks,
Roland

Re: [PATCH v2] Remove union wait

[Florian Weimer]

On 04/11/2016 11:55 PM, Roland McGrath wrote:

> But I will point out that the claim that the deprecation warnings are
> just as bad as outright removal is not really right.  Packagers or
> system-builders or whoever can add -Wno-error=deprecated to the
> compilation flags for a particular package via configure, or for an
> entire package-building system in that system's configuration for
> building all packages (a la redhat-rpm-config), etc.

There are so many build systems out there.  Figuring out how to disable 
-Werror is often more complicated than patching sources to get rid of 
the warning (especially if there are few places where the warning 
triggers, and the required change is straightforward).  Some projects do 
this deliberately to prevent warnings from accumulating.

At scale (for an entire distribution which has just rebased glibc), 
there is sometimes little difference between the two approaches to get 
rid of the deprecation warning.

Florian

Re: [PATCH v2] Remove union wait

[Roland McGrath]

I don't dispute those notions as likely realities in many situations.
I just dispute the notion that they are universal absolutes that mean
the middle way is clearly useless to everyone and can be disregarded
out of hand.

Re: [PATCH v2] Remove union wait

[Florian Weimer]

On 04/12/2016 08:01 PM, Roland McGrath wrote:
> I don't dispute those notions as likely realities in many situations.
> I just dispute the notion that they are universal absolutes that mean
> the middle way is clearly useless to everyone and can be disregarded
> out of hand.

Absolutely true in general.  Based on available data, I just don't think 
staged deprecation is the best approach for dealing with union wait.

Another oddity: Portable software already has a check for the 
availability of union wait.  A deprecated union wait type will still 
pass the configure test, and cause deprecation warnings (possibly 
treated as errors) later in the build.  Direct removal does not.

Florian

Re: [PATCH v2] Remove union wait

[Carlos O'Donell]

On 04/12/2016 03:20 PM, Florian Weimer wrote:
> On 04/12/2016 08:01 PM, Roland McGrath wrote:
>> I don't dispute those notions as likely realities in many
>> situations. I just dispute the notion that they are universal
>> absolutes that mean the middle way is clearly useless to everyone
>> and can be disregarded out of hand.
> 
> Absolutely true in general.  Based on available data, I just don't
> think staged deprecation is the best approach for dealing with union
> wait.
> 
> Another oddity: Portable software already has a check for the
> availability of union wait.  A deprecated union wait type will still
> pass the configure test, and cause deprecation warnings (possibly
> treated as errors) later in the build.  Direct removal does not.

tldr; I do not object to removing union wait in 2.24.

My initial objection was based on the premise that we had no data
about which distribution packages may or may not fail.

I think Andreas and Florian have done a good job of showing that
deprecation in a single cycle is possible given the results of
the openSUSE:Factory build.

The final argument is about giving users enough time to change
their software. In this case the solution is sufficiently simple
that removing it right away seems acceptable. Unlike the complex
changes we did for _DEFAULT_SOURCE which were staged on purpose
to help users transition.

-- 
Cheers,
Carlos.

Re: [PATCH v2] Remove union wait

[Carlos O'Donell]

On 04/11/2016 05:55 PM, Roland McGrath wrote:
> I still don't have much opinion either way about staged deprecation.
> Since few others have opined, I think the decision now is up to
> whether Carlos changes his mind.  

tldr; I don't object.

As I mentioned down-thread, we had no data about breakage, now we have
a full openSUSE:Factory build and Andreas and Florian have addressed
all the issues that were seen. With this data in hand I don't object
to a single cycle removal.

At the end of the day I feel that it's a sufficiently simple change
that single cycle removal is fine. Any more and we're probably wasting
our own community resources tracking and making sure we actually
deprecate the interface and don't forget (see Zach Weinberg's
excellent paper: "A Maintenance ProgrammerÂ’s View of GCC" talking
about incomplete transitions).

-- 
Cheers,
Carlos.

[1] http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.121.7838&rep=rep1&type=pdf

Re: [PATCH v2] Remove union wait

[Roland McGrath]

Thanks, Carlos.  With that approval, I think Florian's last patch
can go in as is.