Re: standards/51603: WIFCONTINUED()==true always implies WIFSTOPPED()==true in the current implementation

[David Holland <dholland-bugs%netbsd.org@localhost>]

The following reply was made to PR standards/51603; it has been noted by GNATS.

From: David Holland <dholland-bugs%netbsd.org@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: standards/51603: WIFCONTINUED()==true always implies
 WIFSTOPPED()==true in the current implementation
Date: Sat, 5 Nov 2016 22:26:30 +0000

 On Sat, Nov 05, 2016 at 08:55:00PM +0000, n54%gmx.com@localhost wrote:
  > Current implementation of <sys/wait.h> implies that a status of
  > wait(2)-like functions for WIFCONTINED()==true implies always true.
 
 I don't think that sentence says what you meant it to say :-)
 
  > In the current code of <sys/wait.h>:
 
 So the traditional encoding, dumb as it may be, is as follows:
 
 0000 0000 0000 0000 BBBB BBBB CAAA AAAA
 |-    31..16     -| |-15..8-| 7|-6..0-|
 
  - if A is 0, it means EXITED and B is the exit status.
  - if A is 0x7f, it means STOPPED and B is the stop signal.
  - if A is some other value, SIGNALED and A is the exit signal.
  - if C is 1 it means a core file was dumped.
 
 It seems that when WIFCONTINUED was added it was meant to be indicated
 by having the whole value equal to 0xffff. In the above encoding this
 value indicates STOPPED on signal 255 with a core dump. This isn't
 what we want.
 
 ISTM that the best way to fix this is to add this rule after the rule
 for STOPPED:
 
   - if A is 0x7e, it means CONTINUED.
 
 (since I guess there's no need to indicate a continue signal)
 
 Since there aren't 126 signals, reserving another value will not break
 the existing logic.
 
 Naive code will then interpret the new WIFSTOPPED as SIGNALED with
 signal 126 but naive code that didn't ask for them shouldn't be
 getting WIFSTOPPED notices. With this change one won't get the wrong
 answer by testing WIFSIGNALED before WIFSTOPPED, and that seems like
 the important part.
 
 However, if that's not good enough we need to use more bits in the
 upper half of the word, because every possible combination of the
 lower bits already means something.
 
 One possible way to do this is to make 0x10000 a flag indicating
 WIFSTOPPED, and then if that bit's 0 revert to the rest of the logic.
 
 A second way is to make another field D that's the next 4 or 8 bits,
 and use that to indicate what happened and then also set the other
 fields according to the traditional encoding, for compatibility.
 
 I don't think either of these is a good plan; this encoding is, though
 historical, silly, and if we're going to change it we might as well
 just change it, insert the compat code needed, and not have to think
 about it again the next time the Linux world or the POSIX committee
 thinks up another silly thing to overload wait() with.
 
 Here's what I would propose as a new encoding:
 
   - bits 0-7 indicate what happened
   - bits 8-31 hold the accompanying value, if any.
 
 Because POSIX insists that the exit status be truncated to 8 bits,
 that would produce the following implementation:
 
   #define _WSTATUS(x) (_W_INT(x) & 0xff)
   #define _WVALUE(x)  ((int)((unsigned)_W_INT(x) >> 8))
   #define _WMAKE(v, s) ((((unsigned)(v)v) << 8) | (s))
 
   #define _WEXITED    0
   #define _WSIGNALED  1
   #define _WCORED     2
   #define _WSTOPPED   3
   #define _WCONTINUED 4
   /* add more as people make stuff up */
 
   #define WIFEXITED(x) (_WSTATUS(x) == _WEXITED)
   #define WIFSIGNALED(x) (_WSTATUS(x) == _WEXITED || _WSTATUS(x) == _WCORED)
   #define WCOREDUMP(x) (_WSTATUS(x) == _WCORED)
   #define WIFSTOPPED(x) (_WSTATUS(x) == _WSTOPPED)
   #define WIFCONTINUED(x) (_WSTATUS(x) == _WCONTINUED)
 
   #ifdef _POSIX_MISTAKE_SOURCE
   #define WEXITSTATUS(x) (_WVALUE(x) & 0xff)
   #else
   #define WEXITSTATUS(x) _WVALUE(x)
   #endif
   #define WTERMSIG(x)    _WVALUE(x)
   #define WSTOPSIG(x)    _WVALUE(x)
 
 although the implementation of W_EXITCODE becomes messy:
 
   #define W_EXITCODE(ret, sig) \
 	((sig)==0 ? _WMAKE(ret, _WEXITED) : _WAKE(sig, _WSIGNALED))
 
 and also doesn't interact well with _WCORED, so it would be better to
 replace it with
 
   #define W_EXITEDCODE(ret) _WMAKE(ret, _WEXITED)
   #define W_SIGNALEDCODE(sig) _WMAKE(sig, _WSIGNALED)
   #define W_COREDCODE(sig) _WMAKE(sig, _WCORED)
   #define W_STOPCODE(sig) _WMAKE(sig, _WSTOPPED)
   #define W_CONTINUEDCODE() _WMAKE(0, _WCONTINUED)
 
 but as (AFAIK) W_EXITCODE is not standard there's no big problem with
 this.
 
  > #if !( defined(_XOPEN_SOURCE) || defined(_NETBSD_SOURCE) ) || defined(_KERNEL)
  > #define _W_INT(i)       (i)
  > #else
  > #define _W_INT(w)       (*(int *)(void *)&(w))  /* convert union wait to int */
  > #endif
 
 Unrelatedly, this should go away. Any code floating around still using
 union wait needs to be patched already. union wait's been deprecated
 for more than twenty years.
 
 -- 
 David A. Holland
 dholland%netbsd.org@localhost