Re: [PATCH] Allow usage of union wait for wait() functions and macros

[Christopher Faylor <cgf-use-the-mailinglist-please at cygwin dot com>]

On Thu, Oct 06, 2011 at 01:03:41PM +0200, Christian Franke wrote:
>Christopher Faylor wrote:
>> On Wed, Oct 05, 2011 at 11:17:58PM +0200, Christian Franke wrote:
>>> Christopher Faylor wrote:
>>>> On Wed, Oct 05, 2011 at 12:57:44PM +0200, Christian Franke wrote:
>>>>> ...
>>>>> diff --git a/winsup/cygwin/include/cygwin/wait.h b/winsup/cygwin/include/cygwin/wait.h
>>>>> index bed81b7..e4edba2 100644
>>>>> --- a/winsup/cygwin/include/cygwin/wait.h
>>>>> +++ b/winsup/cygwin/include/cygwin/wait.h
>>>>> @@ -1,6 +1,6 @@
>>>>> /* cygwin/wait.h
>>>>>
>>>>> -   Copyright 2006, 2009 Red Hat, Inc.
>>>>> +   Copyright 2006, 2009, 2011 Red Hat, Inc.
>>>>>
>>>>> This file is part of Cygwin.
>>>>>
>>>>> @@ -16,6 +16,9 @@ details. */
>>>>> #define WCONTINUED 8
>>>>> #define __W_CONTINUED	0xffff
>>>>>
>>>>> +/* Will be redefined in sys/wait.h.  */
>>>>> +#define __wait_status_to_int(w)  (w)
>>>>> +
>>>> Why is this necessary?  It doesn't look like it is ever expanded in cygwin/wait.h.
>>> This would be needed if cygwin/wait.h is included separately without
>>> sys/wait.h
>>> (e.g. stdlib.h ->  cygwin/stdlib.h ->  cygwin/wait.h)
>>> and some W*() macro is actually used.
>>>
>>>
>>>> If a redefinition is necessary why not put it all in one place?
>>> The W*() macros and union wait are closely related. So a probably better
>>> approach would be to move union wait to cygwin/wait.h and define
>>> __wait_status_to_int() only there.
>>>
>>> But then C++ compile may fail because cygwin/wait.h is sometimes
>>> included indirectly inside an extern "C" block:
>>> w32api/shlobj.h ->  extern "C" { w32api/ole2.h ...->  stdlib.h ...->
>>> cygwin/wait.h }
>> Ok.  I see that Linux's use of similar macros is convoluted too.
>> I really would rather keep this all together but I guess it isn't
>> possible without redesigning stdlib.h and */wait.h.
>
>Linux uses the following approach for the C++ case:
>
>typedef void *__wait_status_ptr_t;
>#define __wait_status_to_int(w)  (*(int*)&(w))
>
>This prevents the C++ inline function problem but is not typesafe at all.
>
>Meantime I found out that the inline functions can be used even if a 
>file is included in an extern "C" block. It is possible to nest another 
>extern "C++" block:
>
>// w32api/shlobj.h:
>extern "C" {
>#include ...
>   ...
>   // cygwin/wait.h:
>   ...
>   #ifdef __cplusplus
>   extern "C++" {
>   inline int __wait_status_to_int(int __status) { .... }
>   inline int __wait_status_to_int(const union wait &__status) { .... }
>   }
>   #endif
>
>}
>
>So it would be possible to move union wait and related functions to 
>cygwin/wait.h. Possible new problem: This pollutes the stdlib.h 
>namespace with 'union wait' and '#define w_termsig' etc..
>
>
>>>> Also since Cygwin is C++ why do you need the __INSIDE_CYGWIN__ here?
>>> There are still ten *.c files in winsup/cygwin :-)
>> % cd winsup/cygwin
>> % grep wait.h **/*.c
>> %
>
>OK, __INSIDE_CYGWIN__ is not needed here in practice (but possibly in 
>theory :-)

I would rather see as little __INSIDE_CYGWIN__ as possible
in external headers.

>Would the patch with __INSIDE_CYGWIN__ removed be GTG?

Yes.

cgf