Re: [libvirt] [PATCH] virsh: change wexitstatus order to allow compilation on mac osx

[Eric Blake]

On 09/15/2010 12:41 PM, Bruno Haible wrote:
> > Since WEXITSTATUS should be usable on constants
>
> Who says that? POSIX [1] says that the argument of WEXITSTATUS has to be
> an integer value returned by wait() or waitpid() function in the *stat_loc
> location.

Interesting argument - and I can agree that by that reading, Darwin's 
headers comply with POSIX even when _POSIX_C_SOURCE is not provided, 
since the only way to get at a *stat_loc value is via an lvalue.

> > it is a bug in their headers, and one that Gnulib should be
> > able to work around.
>
> I disagree. It is a crazy idea to call WEXITSTATUS(0) if wait() or waitpid()
> hasn't return a *stat_loc value of 0. Therefore, almost all correct programs
> call WEXITSTATUS on a variable that is usually of type 'int'.

Taking that argument further: given that the target audience of gnulib 
is people that are expecting basic POSIX conformance, you can argue that 
all programs _that use gnulib_ will pass only int, and never 'union 
wait', to WEXITSTATUS.  But if we go that far, and always pass an int, 
then why should we care if it is an lvalue or not?  Most of the gnulib 
header overrides exist to make it easier to port programs developed 
against glibc to other platforms, and since glibc accepts constant 
arguments, why shouldn't we guarantee that gnu extension elsewhere?

> It is a bug in their headers that you can't write WEXITSTATUS(get_last_stat()),
> but it is easy enough to work around and rarely occurs.
>
> And the workaround would have to be tricky: If you wanted to write an override
> for the WEXITSTATUS macro, how would you write it in a way that it supports
> 'int' arguments but also 'union wait' arguments - for old BSD compatibility -,
> without taking the address of the argument?

I agree that _supporting 'union wait'_ would require non-portable use of 
constructs such as gcc's __typeof__.  However, I argue that there's _no_ 
need for gnulib to worry about old BSD compatibility, therefore, I 
respectfully disagree with your assertion that it would be hard to fix 
in the gnulib override of <sys/wait.h>.  I do concede, however, that 
Darwin's <sys/wait.h> should continue expecting 'union wait' instead of 
'int' even if gnulib can afford to assume a tighter restriction of 'int' 
only.

Given that this has sparked a debate, I think the sanest thing to do for 
now is to merely document the issue, and mention that gnulib (at least 
for the time being) does not work around it, while still leaving that 
option for the future.  At any rate, it feels a bit silly to add a 
'sys_wait-gnu' module merely to guarantee the GNU semantics of 
WEXITSTATUS; either 'sys_wait' (and 'stdlib') should do this always, or 
we merely rely on programmers avoiding the issue in their own code in 
the first place.  Therefore, I'm pushing this:

diff --git i/ChangeLog w/ChangeLog
index 01d353a..4b63a27 100644
--- i/ChangeLog
+++ w/ChangeLog
@@ -1,5 +1,10 @@
 2010-09-15  Eric Blake  <address@hidden>

+       sys/wait: mention issue with WEXITSTATUS(constant)
+       * doc/posix-headers/sys_wait.texi (sys/wait.h): Document the
+       issue.
+       * doc/posix-headers/stdlib.texi (stdlib.h): Likewise.
+
        strnlen: add tests
        * modules/strnlen-tests: New file.
        * tests/test-strnlen.c: Likewise.
diff --git i/doc/posix-headers/stdlib.texi w/doc/posix-headers/stdlib.texi
index b607438..c9b6cb4 100644
--- i/doc/posix-headers/stdlib.texi
+++ w/doc/posix-headers/stdlib.texi
@@ -21,4 +21,8 @@ stdlib.h

 Portability problems not fixed by Gnulib:
 @itemize
address@hidden
+System status macros such as @code{WEXITSTATUS} require an lvalue
+argument on some platforms.
+MacOS 10.5.
 @end itemize
diff --git i/doc/posix-headers/sys_wait.texi 
w/doc/posix-headers/sys_wait.texi
index cb3fe2d..d309fc4 100644
--- i/doc/posix-headers/sys_wait.texi
+++ w/doc/posix-headers/sys_wait.texi
@@ -14,4 +14,8 @@ sys/wait.h

 Portability problems not fixed by Gnulib:
 @itemize
address@hidden
+System status macros such as @code{WEXITSTATUS} require an lvalue
+argument on some platforms.
+MacOS 10.5.
 @end itemize

--
Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org