Re: [Qemu-devel] [PATCH v2 4/5] w32: Replace Windows specific data types in common header files

[Stefan Weil <sw@weilnetz.de>]

Am 10.03.2014 16:17, schrieb Stefan Hajnoczi:
> On Fri, Mar 07, 2014 at 11:17:46PM +0100, Stefan Weil wrote:
>> diff --git a/include/qemu/thread-win32.h b/include/qemu/thread-win32.h
>> index 7ade61a..b8b8e61 100644
>> --- a/include/qemu/thread-win32.h
>> +++ b/include/qemu/thread-win32.h
>> @@ -1,24 +1,35 @@
>>  #ifndef __QEMU_THREAD_WIN32_H
>>  #define __QEMU_THREAD_WIN32_H 1
>> -#include "qemu/winapi.h"
>> +
>> +/* WinCriticalSection is a substitute for CRITICAL_SECTION and
>> + * introduced here to avoid dependencies on windows.h. */
>> +
>> +typedef struct {
>> +    WinHandle DebugInfo;
>> +    WinLong LockCount;
>> +    WinLong RecursionCount;
>> +    WinHandle OwningThread;
>> +    WinHandle LockSemaphore;
>> +    WinULong *SpinCount;
>> +} WinCriticalSection;
> 
> This is taking it a bit far.  Avoiding includes for the scalar types
> seems okay but duplicating struct definitions makes me wonder how far
> we'll go to reduce compile times.  (Plus wouldn't we have the same kind
> of copyright/license issues that mingw and other projects need to be
> very careful about when reimplementing Windows headers?)


I don't think that we have a copyright or license issue here because we
don't use names which were invented by MS. They have a copyright on
win32 (that's why I typically use w32 or wxx), but not on WinXXX AFAIK.
Our existing files with names using win32 might be a problem, although I
doubt that anybody will complain.

Instead of defining a struct, WinCriticalSection could also be an array
which is sufficiently large to store a critical section and which uses
the correct alignment. Do you think that would be a better solution?

> I guess the problem is that qemu-thread.h is included in a lot of places
> and you wish to avoid including <windows.h>.  Still, I would leave this
> one out.
> 
> Stefan
> 

As you noticed, the problem is include/qemu/thread.h which includes
include/qemu/thread-win32.h to define QemuMutex. Unfortunately,
QemuMutex is used by value in include/block/aio.h and in
include/exec/cpu-all.h. Most QEMU files depend on these two files.
Breaking the dependencies of all those files on windows.h is essential
for my patch series. I see only three solutions:

* Leave the code as it is. That implies long compile time for MinGW
builds and name space pollution because nearly every compilation needs
windows.h. This last point is the reason for two existing hacks and one
more hack which is still needed (both Peter and I sent patches for it),
and we have a realistic chance to need future hacks from time to time.

* Break the dependency on windows.h by using QEMU data types instead of
Windows API data types.

* Break the dependency on windows.h by avoiding the use of certain QEMU
data types (especially QemuMutex) by value, because those QEMU data
types use Windows data types.

I must admit that I tried that third solution and gave up after a while.

What do you suggest to do? For me, any of the three alternatives is
fine. I have no personal use for QEMU on Windows, nor do I need it for
my professional work any longer.

Stefan