Re: [PATCH] Avoid polluting cygwin namespace.

[Eric Blake]

On 04/06/2010 03:57 PM, Bruno Haible wrote:
> Hi Eric,
> 
>>    cygwin*)
>>      FAULT_CONTEXT='CONTEXT'
>> -    FAULT_CONTEXT_INCLUDE='#include <windows.h>'
>> +    # CONTEXT is defined in <windows.h>, but cygwin programs should not
>> +    # normally inspect the contents of CONTEXT, and the outright inclusion
>> +    # of <windows.h> pollutes the namespace.
>> +    FAULT_CONTEXT_INCLUDE='typedef struct _CONTEXT CONTEXT;'
> 
> This patch will break in two ways. I'm therefore opposed to it.
> 
> 1) If the user does a #include <windows.h> before or after <sigsegv.h>,
>    he'll get a compilation error about a redefinition of a type.
> 
>    error: redefinition of typedef ‘CONTEXT’
> 
>    In C++, duplicated redundant typedefs are harmless, but not in C.

That can be worked around; here's an alternative approach:

FAULT_CONTEXT='struct _CONTEXT'
FAULT_CONTEXT_INCLUDE='struct _CONTEXT; /* include <windows.h> to see
inside */'

Then you can include <windows.h> before or after <sigsegv.h> if you need
to access the guts, and aren't forced to include it when you don't care
about the guts; and we are no longer repeating the typedef of CONTEXT.

> 
> 2) The users of libsigsegv need to access the field of the FAULT_CONTEXT.
>    That's what it is provided for, in the first place. For example, GNU clisp
>    does in src/spvw_sigsegv.d:
> 
>      stackoverflow_context_t scp = (stackoverflow_context_t) arg1;
>      ...
>      #if defined(WIN32_NATIVE) || defined(UNIX_CYGWIN32)
>       #ifdef I80386
>        if (scp) { setSTACK(STACK = (gcv_object_t*)(scp->Ebx)); }
>       #endif
>      #endif
> 
>    This would be impossible if stackoverflow_context_t was an opaque type.

If you are already using windows-specific #ifdefs, then it doesn't hurt
to add another one in src/spvw_sigsegv.d:

#if defined WIN32_NATIVE || defined UNIX_CYGWIN32
# include <windows.h>
#endif

> 
> Instead, let's look more closely at the original problem: <windows.h>
> defines WCHAR to a type, whereas grep/src/dfa.h uses it as an enum item.

<windows.h> pollutes much more than WCHAR.  For mingw, this is expected
(in fact, it would be very surprising to have a mingw program that
doesn't already plan on using windows.h as-is).  But for cygwin, it goes
counter to the goals of cygwin of providing a Unix-y environment.  True,
cygwin does not yet provide ucontext_t, but when it eventually does, GNU
clisp should be using ucontext_t on cygwin rather than CONTEXT at that
point, anyways.  So getting rid of dragging in <windows.h> is a good
thing for cygwin portability.

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

signature.asc
Description: OpenPGP digital signature