[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH libvirt 2/6] include winsock2.h before windows.h



On 01/25/2012 03:16 PM, Marc-André Lureau wrote:
>> NACK.  I agree that things should be fixed, but the fix should live
>> in
>> the problematic "threads.h" inclusion pattern, not in the downstream
>> virkeepalive.c.
> 
> I agree, but ff we follow that reasoning, windows.h should include winsock2.h, shouldn't it?

If it were POSIX, the answer is yes - all POSIX headers are required to
be idempotent and self-contained, so any header that requires some other
header to be included first, instead of doing the prereq includes under
the hood itself, is broken.  And gnulib already does a great job of
fixing the (surprisingly large number of) systems that fail this POSIX
header litmus test.

But we're talking about Windows, so we have to follow Microsoft
conventions.  And if Microsoft requires <winsock2.h> to be included
before <windows.h>, then mingw64 is doing the right thing in mimicking
that convention; at which point, we can't insist on "fixing"
<windows.h>.  But we _can_ insist on fixing libvirt's use of Windows
headers so that _only_ the windows-specific files have to know about the
dependency ordering constraints, and all other libvirt files can use any
libvirt header without having to think about inherited dependency
constraints.  threads-win32.h is our windows-specific header that was
triggering the problem, so that's the point in the chain worth fixing,
so that normal files, like virkeepalive.c don't have to think about "if
I include both "threads.h" and <unistd.h>, am I introducing an ordering
constraint on windows platforms?"

>> Does this alternate patch work for you?  (One of these days, I need
>> to
>> install the mingw64 cross-compiler, so I can test it myself.)
> 
> Yes, that works too.

Glad to hear it; I've pushed the alternate patch.

> 
> Btw, I don't mind testing patches, but using mingw64 is almost a 'yum install' away :) 

What repo?  The mingw64 compiler isn't in Fedora 16 proper.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]