qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/5] w32: Reduce dependency on Windows API
@ 2014-03-07 22:17 Stefan Weil
  2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 1/5] w32: Add and use intermediate include file for windows.h Stefan Weil
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Stefan Weil @ 2014-03-07 22:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, stefanha, peter.maydell

The first 4 patches reduce the number of files which depend on
windows.h by about 90 percent.

This reduces the compilation time, allows
removing some hacks and avoids name space pollution.

Patch 5 is optional and new here.

Changes in v2:

* Change name of new include file include/qemu/winapi.h
  (suggested by Paolo Bonzini)

* Don't replace Win32 data types in block/raw-aio.h
  (suggested by Kevin Wolf)

* Replace Win32 data types in central header files by
  new QEMU data types instead of basic C data types
  (suggested by Peter Maydell and Stefan Hajnoczi)

[PATCH v2 1/5] w32: Add and use intermediate include file for windows.h
[PATCH v2 2/5] w32: Move inline function from header file to C source
[PATCH v2 3/5] w32: Reduce dependencies in sysemu/os-win32.h
[PATCH v2 4/5] w32: Replace Windows specific data types in common
[PATCH v2 5/5] block: Review include statements for winioctl.h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH v2 1/5] w32: Add and use intermediate include file for windows.h
  2014-03-07 22:17 [Qemu-devel] [PATCH v2 0/5] w32: Reduce dependency on Windows API Stefan Weil
@ 2014-03-07 22:17 ` Stefan Weil
  2014-03-10  8:56   ` Markus Armbruster
  2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 2/5] w32: Move inline function from header file to C source Stefan Weil
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Stefan Weil @ 2014-03-07 22:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, Anthony Liguori, Stefan Weil, Gerd Hoffmann,
	stefanha, Jan Kiszka, pbonzini

Including windows.h from the new file include/qemu/winapi.h allows
better tracking of the files which depend on the Windows API.

1864 *.o files depend on windows.h in a typical build, only 88 *.o files
don't.

The windows.h specific macro WIN32_LEAN_AND_MEAN is now defined in the new
file and no longer part of the QEMU_CFLAGS. A hack in ui/sdl.c can be
removed after this change.

WINVER is still needed for all compilations with MinGW, so it cannot be
defined in the new file. Replace its numeric value by a symbolic value to
show which API is requested.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 audio/audio_win_int.c         |    2 +-
 audio/dsoundaudio.c           |    2 +-
 audio/winwaveaudio.c          |    2 +-
 block.c                       |    2 +-
 block/raw-win32.c             |    2 +-
 block/win32-aio.c             |    2 +-
 configure                     |    2 +-
 include/qemu/event_notifier.h |    2 +-
 include/qemu/sockets.h        |    2 +-
 include/qemu/thread-win32.h   |    2 +-
 include/qemu/winapi.h         |   22 ++++++++++++++++++++++
 include/sysemu/os-win32.h     |    2 +-
 net/tap-win32.c               |    2 +-
 os-win32.c                    |    2 +-
 qga/channel-win32.c           |    2 +-
 qga/commands-win32.c          |    2 +-
 qga/service-win32.c           |    2 +-
 qga/service-win32.h           |    2 +-
 qga/vss-win32.c               |    2 +-
 qga/vss-win32/vss-common.h    |    2 +-
 slirp/slirp.h                 |    2 +-
 translate-all.c               |    2 +-
 ui/sdl.c                      |    3 ---
 util/oslib-win32.c            |    2 +-
 24 files changed, 44 insertions(+), 25 deletions(-)
 create mode 100644 include/qemu/winapi.h

diff --git a/audio/audio_win_int.c b/audio/audio_win_int.c
index e132405..0e9f2a4 100644
--- a/audio/audio_win_int.c
+++ b/audio/audio_win_int.c
@@ -3,7 +3,7 @@
 #include "qemu-common.h"
 
 #define AUDIO_CAP "win-int"
-#include <windows.h>
+#include "qemu/winapi.h"
 #include <mmsystem.h>
 
 #include "audio.h"
diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index e2d89fd..cf779bf 100644
--- a/audio/dsoundaudio.c
+++ b/audio/dsoundaudio.c
@@ -32,7 +32,7 @@
 #define AUDIO_CAP "dsound"
 #include "audio_int.h"
 
-#include <windows.h>
+#include "qemu/winapi.h"
 #include <mmsystem.h>
 #include <objbase.h>
 #include <dsound.h>
diff --git a/audio/winwaveaudio.c b/audio/winwaveaudio.c
index 8dbd145..f11f5ba 100644
--- a/audio/winwaveaudio.c
+++ b/audio/winwaveaudio.c
@@ -7,7 +7,7 @@
 #define AUDIO_CAP "winwave"
 #include "audio_int.h"
 
-#include <windows.h>
+#include "qemu/winapi.h"
 #include <mmsystem.h>
 
 #include "audio_win_int.h"
diff --git a/block.c b/block.c
index 38bbdf3..409cbd8 100644
--- a/block.c
+++ b/block.c
@@ -47,7 +47,7 @@
 #endif
 
 #ifdef _WIN32
-#include <windows.h>
+#include "qemu/winapi.h"
 #endif
 
 struct BdrvDirtyBitmap {
diff --git a/block/raw-win32.c b/block/raw-win32.c
index ae1c8e6..95b27a5 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -23,13 +23,13 @@
  */
 #include "qemu-common.h"
 #include "qemu/timer.h"
+#include "qemu/winapi.h"        /* HANDLE (in raw-aio.h) */
 #include "block/block_int.h"
 #include "qemu/module.h"
 #include "raw-aio.h"
 #include "trace.h"
 #include "block/thread-pool.h"
 #include "qemu/iov.h"
-#include <windows.h>
 #include <winioctl.h>
 
 #define FTYPE_FILE 0
diff --git a/block/win32-aio.c b/block/win32-aio.c
index 5d1d199..dbcc6bc 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -23,13 +23,13 @@
  */
 #include "qemu-common.h"
 #include "qemu/timer.h"
+#include "qemu/winapi.h"        /* HANDLE (in raw-aio.h) */
 #include "block/block_int.h"
 #include "qemu/module.h"
 #include "block/aio.h"
 #include "raw-aio.h"
 #include "qemu/event_notifier.h"
 #include "qemu/iov.h"
-#include <windows.h>
 #include <winioctl.h>
 
 #define FTYPE_FILE 0
diff --git a/configure b/configure
index 8689435..daf6f69 100755
--- a/configure
+++ b/configure
@@ -682,7 +682,7 @@ fi
 if test "$mingw32" = "yes" ; then
   EXESUF=".exe"
   DSOSUF=".dll"
-  QEMU_CFLAGS="-DWIN32_LEAN_AND_MEAN -DWINVER=0x501 $QEMU_CFLAGS"
+  QEMU_CFLAGS="-DWINVER=_WIN32_WINNT_WINXP $QEMU_CFLAGS"
   # enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later)
   QEMU_CFLAGS="-D__USE_MINGW_ANSI_STDIO=1 $QEMU_CFLAGS"
   LIBS="-lwinmm -lws2_32 -liphlpapi $LIBS"
diff --git a/include/qemu/event_notifier.h b/include/qemu/event_notifier.h
index 88b57af..bdca689 100644
--- a/include/qemu/event_notifier.h
+++ b/include/qemu/event_notifier.h
@@ -16,7 +16,7 @@
 #include "qemu-common.h"
 
 #ifdef _WIN32
-#include <windows.h>
+#include "qemu/winapi.h"
 #endif
 
 struct EventNotifier {
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 45588d7..a41d453 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -3,7 +3,7 @@
 #define QEMU_SOCKET_H
 
 #ifdef _WIN32
-#include <windows.h>
+#include "qemu/winapi.h"
 #include <winsock2.h>
 #include <ws2tcpip.h>
 
diff --git a/include/qemu/thread-win32.h b/include/qemu/thread-win32.h
index 3d58081..7ade61a 100644
--- a/include/qemu/thread-win32.h
+++ b/include/qemu/thread-win32.h
@@ -1,6 +1,6 @@
 #ifndef __QEMU_THREAD_WIN32_H
 #define __QEMU_THREAD_WIN32_H 1
-#include "windows.h"
+#include "qemu/winapi.h"
 
 struct QemuMutex {
     CRITICAL_SECTION lock;
diff --git a/include/qemu/winapi.h b/include/qemu/winapi.h
new file mode 100644
index 0000000..d4cfaaa
--- /dev/null
+++ b/include/qemu/winapi.h
@@ -0,0 +1,22 @@
+/*
+ * QEMU interface to the Windows API
+ *
+ * Copyright (c) 2014 Stefan Weil
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2
+ * or later.  See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_WINAPI_H
+#define QEMU_WINAPI_H
+
+/* Don't include some less frequently used APIs. */
+#define WIN32_LEAN_AND_MEAN
+
+#if !defined(WINVER)
+# error Add -DWINVER=_WIN32_WINNT_WINXP to compiler options
+#endif
+
+#include <windows.h>
+
+#endif /* QEMU_WINAPI_H */
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index bf8523a..1d6494a 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -26,7 +26,7 @@
 #ifndef QEMU_OS_WIN32_H
 #define QEMU_OS_WIN32_H
 
-#include <windows.h>
+#include "qemu/winapi.h"
 #include <winsock2.h>
 
 /* Workaround for older versions of MinGW. */
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 8aee611..52e6143 100644
--- a/net/tap-win32.c
+++ b/net/tap-win32.c
@@ -35,7 +35,7 @@
 #include "sysemu/sysemu.h"
 #include "qemu/error-report.h"
 #include <stdio.h>
-#include <windows.h>
+#include "qemu/winapi.h"
 #include <winioctl.h>
 
 //=============
diff --git a/os-win32.c b/os-win32.c
index 5f95caa..f6ec112 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -22,7 +22,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
-#include <windows.h>
+#include "qemu/winapi.h"
 #include <mmsystem.h>
 #include <unistd.h>
 #include <fcntl.h>
diff --git a/qga/channel-win32.c b/qga/channel-win32.c
index 0d5e5f5..25c3257 100644
--- a/qga/channel-win32.c
+++ b/qga/channel-win32.c
@@ -2,7 +2,7 @@
 #include <stdio.h>
 #include <stdbool.h>
 #include <glib.h>
-#include <windows.h>
+#include "qemu/winapi.h"
 #include <errno.h>
 #include <io.h>
 #include "qga/guest-agent-core.h"
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 0ee07b6..48cc819 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -11,8 +11,8 @@
  * See the COPYING file in the top-level directory.
  */
 
+#include "qemu/winapi.h" /* OpenProcessToken, ... */
 #include <glib.h>
-#include <wtypes.h>
 #include <powrprof.h>
 #include "qga/guest-agent-core.h"
 #include "qga/vss-win32.h"
diff --git a/qga/service-win32.c b/qga/service-win32.c
index aef41f0..b22ae67 100644
--- a/qga/service-win32.c
+++ b/qga/service-win32.c
@@ -13,7 +13,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <glib.h>
-#include <windows.h>
+#include "qemu/winapi.h"
 #include "qga/service-win32.h"
 
 static int printf_win_error(const char *text)
diff --git a/qga/service-win32.h b/qga/service-win32.h
index 3b9e870..bd401f2 100644
--- a/qga/service-win32.h
+++ b/qga/service-win32.h
@@ -13,7 +13,7 @@
 #ifndef QGA_SERVICE_H
 #define QGA_SERVICE_H
 
-#include <windows.h>
+#include "qemu/winapi.h"
 
 #define QGA_SERVICE_DISPLAY_NAME "QEMU Guest Agent"
 #define QGA_SERVICE_NAME         "qemu-ga"
diff --git a/qga/vss-win32.c b/qga/vss-win32.c
index 24c4288..c6970d2 100644
--- a/qga/vss-win32.c
+++ b/qga/vss-win32.c
@@ -11,7 +11,7 @@
  */
 
 #include <stdio.h>
-#include <windows.h>
+#include "qemu/winapi.h"
 #include "qga/guest-agent-core.h"
 #include "qga/vss-win32.h"
 #include "qga/vss-win32/requester.h"
diff --git a/qga/vss-win32/vss-common.h b/qga/vss-win32/vss-common.h
index ce14e14..9768c1e 100644
--- a/qga/vss-win32/vss-common.h
+++ b/qga/vss-win32/vss-common.h
@@ -15,7 +15,7 @@
 
 #define __MIDL_user_allocate_free_DEFINED__
 #include "config-host.h"
-#include <windows.h>
+#include "qemu/winapi.h"
 #include <shlwapi.h>
 
 /* Reduce warnings to include vss.h */
diff --git a/slirp/slirp.h b/slirp/slirp.h
index e4a1bd4..bb25ab9 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -9,7 +9,7 @@
 
 typedef char *caddr_t;
 
-# include <windows.h>
+# include "qemu/winapi.h"
 # include <winsock2.h>
 # include <ws2tcpip.h>
 # include <sys/timeb.h>
diff --git a/translate-all.c b/translate-all.c
index 1ac0246..c124c07 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -17,7 +17,7 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 #ifdef _WIN32
-#include <windows.h>
+#include "qemu/winapi.h"
 #else
 #include <sys/types.h>
 #include <sys/mman.h>
diff --git a/ui/sdl.c b/ui/sdl.c
index c1a16be..efae4a7 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -22,9 +22,6 @@
  * THE SOFTWARE.
  */
 
-/* Avoid compiler warning because macro is redefined in SDL_syswm.h. */
-#undef WIN32_LEAN_AND_MEAN
-
 #include <SDL.h>
 
 #if SDL_MAJOR_VERSION == 1
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 93f7d35..3fbe72a 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -25,7 +25,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
-#include <windows.h>
+#include "qemu/winapi.h"
 #include <glib.h>
 #include <stdlib.h>
 #include "config-host.h"
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH v2 2/5] w32: Move inline function from header file to C source
  2014-03-07 22:17 [Qemu-devel] [PATCH v2 0/5] w32: Reduce dependency on Windows API Stefan Weil
  2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 1/5] w32: Add and use intermediate include file for windows.h Stefan Weil
@ 2014-03-07 22:17 ` Stefan Weil
  2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 3/5] w32: Reduce dependencies in sysemu/os-win32.h Stefan Weil
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Stefan Weil @ 2014-03-07 22:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, Stefan Weil, stefanha, peter.maydell

A lot of files depend on qemu/timer.h. We don't want that all these files
depend on windows.h, too.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 include/qemu/timer.h     |    8 +-------
 util/qemu-timer-common.c |   11 ++++++++++-
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 7f9a074..19316b7 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -712,14 +712,8 @@ static inline int64_t get_clock_realtime(void)
    also used by simpletrace backend and tracepoints would cause
    an infinite recursion! */
 #ifdef _WIN32
-extern int64_t clock_freq;
 
-static inline int64_t get_clock(void)
-{
-    LARGE_INTEGER ti;
-    QueryPerformanceCounter(&ti);
-    return muldiv64(ti.QuadPart, get_ticks_per_sec(), clock_freq);
-}
+int64_t get_clock(void);
 
 #else
 
diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c
index 95e0847..22b0109 100644
--- a/util/qemu-timer-common.c
+++ b/util/qemu-timer-common.c
@@ -28,7 +28,16 @@
 
 #ifdef _WIN32
 
-int64_t clock_freq;
+#include "qemu/winapi.h" /* QueryPerformanceCounter, ... */
+
+static int64_t clock_freq;
+
+int64_t get_clock(void)
+{
+    LARGE_INTEGER ti;
+    QueryPerformanceCounter(&ti);
+    return muldiv64(ti.QuadPart, get_ticks_per_sec(), clock_freq);
+}
 
 static void __attribute__((constructor)) init_get_clock(void)
 {
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH v2 3/5] w32: Reduce dependencies in sysemu/os-win32.h
  2014-03-07 22:17 [Qemu-devel] [PATCH v2 0/5] w32: Reduce dependency on Windows API Stefan Weil
  2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 1/5] w32: Add and use intermediate include file for windows.h Stefan Weil
  2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 2/5] w32: Move inline function from header file to C source Stefan Weil
@ 2014-03-07 22:17 ` Stefan Weil
  2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 4/5] w32: Replace Windows specific data types in common header files Stefan Weil
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Stefan Weil @ 2014-03-07 22:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, stefanha, Stefan Weil, Max Filippov,
	Anthony Liguori, pbonzini

Most *.o files depend on that file, but many of them don't need windows.h
or winsock2.h. sysemu/os-win32.h only needs some definitions from
winerror.h.

After that change, all files which depend on windows.h or winsock2.h and
which no longer get it indirectly have to be fixed. Use qemu/sockets.h
to get winsock2.h. Add comments to all those new include statements.

The modification in ui/vnc-enc-tight.c is needed temporarily and will be
removed again in the following patch.

Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 coroutine-win32.c           |    1 +
 include/net/eth.h           |    2 +-
 include/sysemu/os-win32.h   |    3 +--
 target-xtensa/xtensa-semi.c |    1 +
 ui/vnc-enc-tight.c          |    1 +
 5 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/coroutine-win32.c b/coroutine-win32.c
index edc1f72..4678d17 100644
--- a/coroutine-win32.c
+++ b/coroutine-win32.c
@@ -24,6 +24,7 @@
 
 #include "qemu-common.h"
 #include "block/coroutine_int.h"
+#include "qemu/winapi.h"   /* CreateFiber, ... */
 
 typedef struct
 {
diff --git a/include/net/eth.h b/include/net/eth.h
index b3273b8..f5a369f 100644
--- a/include/net/eth.h
+++ b/include/net/eth.h
@@ -26,10 +26,10 @@
 #ifndef QEMU_ETH_H
 #define QEMU_ETH_H
 
-#include <sys/types.h>
 #include <string.h>
 #include "qemu/bswap.h"
 #include "qemu/iov.h"
+#include "qemu/sockets.h" /* u_short */
 
 #define ETH_ALEN 6
 
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 1d6494a..d625612 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -26,8 +26,7 @@
 #ifndef QEMU_OS_WIN32_H
 #define QEMU_OS_WIN32_H
 
-#include "qemu/winapi.h"
-#include <winsock2.h>
+#include <winerror.h> /* WSAECONNREFUSED, ... */
 
 /* Workaround for older versions of MinGW. */
 #ifndef ECONNREFUSED
diff --git a/target-xtensa/xtensa-semi.c b/target-xtensa/xtensa-semi.c
index 424253d..ad06f99 100644
--- a/target-xtensa/xtensa-semi.c
+++ b/target-xtensa/xtensa-semi.c
@@ -32,6 +32,7 @@
 #include "cpu.h"
 #include "helper.h"
 #include "qemu/log.h"
+#include "qemu/sockets.h" /* select */
 
 enum {
     TARGET_SYS_exit = 1,
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index e6966ae..94bb002 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -31,6 +31,7 @@
 /* This needs to be before jpeglib.h line because of conflict with
    INT32 definitions between jmorecfg.h (included by jpeglib.h) and
    Win32 basetsd.h (included by windows.h). */
+#include "qemu/winapi.h" /* TODO: workaround, remove */
 #include "qemu-common.h"
 
 #ifdef CONFIG_VNC_PNG
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH v2 4/5] w32: Replace Windows specific data types in common header files
  2014-03-07 22:17 [Qemu-devel] [PATCH v2 0/5] w32: Reduce dependency on Windows API Stefan Weil
                   ` (2 preceding siblings ...)
  2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 3/5] w32: Reduce dependencies in sysemu/os-win32.h Stefan Weil
@ 2014-03-07 22:17 ` Stefan Weil
  2014-03-10 15:17   ` Stefan Hajnoczi
  2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 5/5] block: Review include statements for winioctl.h Stefan Weil
  2014-03-10 15:18 ` [Qemu-devel] [PATCH v2 0/5] w32: Reduce dependency on Windows API Stefan Hajnoczi
  5 siblings, 1 reply; 13+ messages in thread
From: Stefan Weil @ 2014-03-07 22:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, peter.maydell, stefanha, Stefan Weil, Anthony Liguori,
	pbonzini, Andreas Färber

These header files are used by most QEMU source files. If they
depend on windows.h, all those source files do so, too.

All Windows specific data types which are replaced use identical
definitions for the 32 and 64 bit Windows APIs. HANDLE, LONG
and CRITICAL_SECTION are replaced by the compatible types
WinHandle, WinLong and WinCriticalSection.

Add an explicit dependency on qemu/winapi.h for some files which need it.
These sources use the Windows API (see comment after include statement)
and no longer get windows.h indirectly from other header files.

A workaround which was added in the previous patch is no longer needed.

Now 175 *.o files remain which still depend on windows.h.

Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Andreas Färber <afaerber@suse.de>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 cpus.c                        |    4 +++-
 hw/intc/apic.c                |    3 ++-
 include/qemu/event_notifier.h |    8 ++------
 include/qemu/main-loop.h      |    4 ++--
 include/qemu/thread-win32.h   |   29 ++++++++++++++++++++---------
 include/qom/cpu.h             |    2 +-
 include/sysemu/os-win32.h     |    7 +++++++
 ui/vnc-enc-tight.c            |    5 -----
 util/event_notifier-win32.c   |    1 +
 util/qemu-thread-win32.c      |   17 +++++++++--------
 10 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/cpus.c b/cpus.c
index 945d85b..a96eb16 100644
--- a/cpus.c
+++ b/cpus.c
@@ -39,7 +39,9 @@
 #include "qemu/bitmap.h"
 #include "qemu/seqlock.h"
 
-#ifndef _WIN32
+#ifdef _WIN32
+#include "qemu/winapi.h" /* SuspendThread, ... */
+#else
 #include "qemu/compatfd.h"
 #endif
 
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 361ae90..6bb2d78 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -16,12 +16,13 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see <http://www.gnu.org/licenses/>
  */
-#include "qemu/thread.h"
+
 #include "hw/i386/apic_internal.h"
 #include "hw/i386/apic.h"
 #include "hw/i386/ioapic.h"
 #include "hw/pci/msi.h"
 #include "qemu/host-utils.h"
+#include "qemu/thread.h"
 #include "trace.h"
 #include "hw/i386/pc.h"
 #include "hw/i386/apic-msidef.h"
diff --git a/include/qemu/event_notifier.h b/include/qemu/event_notifier.h
index bdca689..c5cf381 100644
--- a/include/qemu/event_notifier.h
+++ b/include/qemu/event_notifier.h
@@ -15,13 +15,9 @@
 
 #include "qemu-common.h"
 
-#ifdef _WIN32
-#include "qemu/winapi.h"
-#endif
-
 struct EventNotifier {
 #ifdef _WIN32
-    HANDLE event;
+    WinHandle event;
 #else
     int rfd;
     int wfd;
@@ -40,7 +36,7 @@ int event_notifier_set_handler(EventNotifier *, EventNotifierHandler *);
 void event_notifier_init_fd(EventNotifier *, int fd);
 int event_notifier_get_fd(EventNotifier *);
 #else
-HANDLE event_notifier_get_handle(EventNotifier *);
+WinHandle event_notifier_get_handle(EventNotifier *);
 #endif
 
 #endif
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 6f0200a..aefdc94 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -152,7 +152,7 @@ typedef void WaitObjectFunc(void *opaque);
  * @func: A function to be called when @handle is in a signaled state.
  * @opaque: A pointer-size value that is passed to @func.
  */
-int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
+int qemu_add_wait_object(WinHandle handle, WaitObjectFunc *func, void *opaque);
 
 /**
  * qemu_del_wait_object: Unregister a callback for a Windows handle
@@ -163,7 +163,7 @@ int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
  * @func: The function that was passed to qemu_add_wait_object.
  * @opaque: A pointer-size value that was passed to qemu_add_wait_object.
  */
-void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque);
+void qemu_del_wait_object(WinHandle handle, WaitObjectFunc *func, void *opaque);
 #endif
 
 /* async I/O support */
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;
 
 struct QemuMutex {
-    CRITICAL_SECTION lock;
-    LONG owner;
+    WinCriticalSection lock;
+    WinLong owner;
 };
 
 struct QemuCond {
-    LONG waiters, target;
-    HANDLE sema;
-    HANDLE continue_event;
+    WinLong waiters, target;
+    WinHandle sema;
+    WinHandle continue_event;
 };
 
 struct QemuSemaphore {
-    HANDLE sema;
+    WinHandle sema;
 };
 
 struct QemuEvent {
-    HANDLE event;
+    WinHandle event;
 };
 
 typedef struct QemuThreadData QemuThreadData;
@@ -28,6 +39,6 @@ struct QemuThread {
 };
 
 /* Only valid for joinable threads.  */
-HANDLE qemu_thread_get_handle(QemuThread *thread);
+WinHandle qemu_thread_get_handle(QemuThread *thread);
 
 #endif
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index d734be8..6626681 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -171,7 +171,7 @@ struct CPUState {
 
     struct QemuThread *thread;
 #ifdef _WIN32
-    HANDLE hThread;
+    WinHandle hThread;
 #endif
     int thread_id;
     uint32_t host_tid;
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index d625612..a23d6aa 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -54,6 +54,13 @@
 # define EWOULDBLOCK  WSAEWOULDBLOCK
 #endif
 
+/* Define substitutes for the Win API types HANDLE, LONG, ULONG.
+ * These types are used to avoid dependencies on windows.h. */
+
+typedef void * WinHandle;
+typedef long WinLong;
+typedef unsigned long WinULong;
+
 #if defined(_WIN64)
 /* On w64, setjmp is implemented by _setjmp which needs a second parameter.
  * If this parameter is NULL, longjump does no stack unwinding.
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 94bb002..2e6fb88 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -27,11 +27,6 @@
  */
 
 #include "config-host.h"
-
-/* This needs to be before jpeglib.h line because of conflict with
-   INT32 definitions between jmorecfg.h (included by jpeglib.h) and
-   Win32 basetsd.h (included by windows.h). */
-#include "qemu/winapi.h" /* TODO: workaround, remove */
 #include "qemu-common.h"
 
 #ifdef CONFIG_VNC_PNG
diff --git a/util/event_notifier-win32.c b/util/event_notifier-win32.c
index 6dbb530..9f3f61d 100644
--- a/util/event_notifier-win32.c
+++ b/util/event_notifier-win32.c
@@ -13,6 +13,7 @@
 #include "qemu-common.h"
 #include "qemu/event_notifier.h"
 #include "qemu/main-loop.h"
+#include "qemu/winapi.h" /* CreateEvent, ... */
 
 int event_notifier_init(EventNotifier *e, int active)
 {
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index 27a5217..9de0136 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -10,11 +10,10 @@
  * See the COPYING file in the top-level directory.
  *
  */
+
+#include "qemu/winapi.h" /* LocalFree, ... */
 #include "qemu-common.h"
 #include "qemu/thread.h"
-#include <process.h>
-#include <assert.h>
-#include <limits.h>
 
 static void error_exit(int err, const char *msg)
 {
@@ -30,18 +29,20 @@ static void error_exit(int err, const char *msg)
 void qemu_mutex_init(QemuMutex *mutex)
 {
     mutex->owner = 0;
-    InitializeCriticalSection(&mutex->lock);
+    /* mutex->lock uses a data type which must fit CRITICAL_SECTION. */
+    g_assert(sizeof(mutex->lock) == sizeof(CRITICAL_SECTION));
+    InitializeCriticalSection((CRITICAL_SECTION *)&mutex->lock);
 }
 
 void qemu_mutex_destroy(QemuMutex *mutex)
 {
     assert(mutex->owner == 0);
-    DeleteCriticalSection(&mutex->lock);
+    DeleteCriticalSection((CRITICAL_SECTION *)&mutex->lock);
 }
 
 void qemu_mutex_lock(QemuMutex *mutex)
 {
-    EnterCriticalSection(&mutex->lock);
+    EnterCriticalSection((CRITICAL_SECTION *)&mutex->lock);
 
     /* Win32 CRITICAL_SECTIONs are recursive.  Assert that we're not
      * using them as such.
@@ -54,7 +55,7 @@ int qemu_mutex_trylock(QemuMutex *mutex)
 {
     int owned;
 
-    owned = TryEnterCriticalSection(&mutex->lock);
+    owned = TryEnterCriticalSection((CRITICAL_SECTION *)&mutex->lock);
     if (owned) {
         assert(mutex->owner == 0);
         mutex->owner = GetCurrentThreadId();
@@ -66,7 +67,7 @@ void qemu_mutex_unlock(QemuMutex *mutex)
 {
     assert(mutex->owner == GetCurrentThreadId());
     mutex->owner = 0;
-    LeaveCriticalSection(&mutex->lock);
+    LeaveCriticalSection((CRITICAL_SECTION *)&mutex->lock);
 }
 
 void qemu_cond_init(QemuCond *cond)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH v2 5/5] block: Review include statements for winioctl.h
  2014-03-07 22:17 [Qemu-devel] [PATCH v2 0/5] w32: Reduce dependency on Windows API Stefan Weil
                   ` (3 preceding siblings ...)
  2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 4/5] w32: Replace Windows specific data types in common header files Stefan Weil
@ 2014-03-07 22:17 ` Stefan Weil
  2014-03-10 15:18 ` [Qemu-devel] [PATCH v2 0/5] w32: Reduce dependency on Windows API Stefan Hajnoczi
  5 siblings, 0 replies; 13+ messages in thread
From: Stefan Weil @ 2014-03-07 22:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, pbonzini, Stefan Weil, stefanha, peter.maydell

block/win32-aio.c does not need it.
Add a comment why it is needed to block/raw-win32.c.

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 block/raw-win32.c |    2 +-
 block/win32-aio.c |    1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/raw-win32.c b/block/raw-win32.c
index 95b27a5..2231a30 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -30,7 +30,7 @@
 #include "trace.h"
 #include "block/thread-pool.h"
 #include "qemu/iov.h"
-#include <winioctl.h>
+#include <winioctl.h>           /* FSCTL_SET_SPARSE, ... */
 
 #define FTYPE_FILE 0
 #define FTYPE_CD     1
diff --git a/block/win32-aio.c b/block/win32-aio.c
index dbcc6bc..a10c16f 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -30,7 +30,6 @@
 #include "raw-aio.h"
 #include "qemu/event_notifier.h"
 #include "qemu/iov.h"
-#include <winioctl.h>
 
 #define FTYPE_FILE 0
 #define FTYPE_CD     1
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/5] w32: Add and use intermediate include file for windows.h
  2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 1/5] w32: Add and use intermediate include file for windows.h Stefan Weil
@ 2014-03-10  8:56   ` Markus Armbruster
  2014-03-10 17:38     ` Stefan Weil
  0 siblings, 1 reply; 13+ messages in thread
From: Markus Armbruster @ 2014-03-10  8:56 UTC (permalink / raw)
  To: Stefan Weil
  Cc: kwolf, peter.maydell, Anthony Liguori, Jan Kiszka, qemu-devel,
	Gerd Hoffmann, stefanha, pbonzini

Stefan Weil <sw@weilnetz.de> writes:

> Including windows.h from the new file include/qemu/winapi.h allows
> better tracking of the files which depend on the Windows API.
>
> 1864 *.o files depend on windows.h in a typical build, only 88 *.o files
> don't.
>
> The windows.h specific macro WIN32_LEAN_AND_MEAN is now defined in the new
> file and no longer part of the QEMU_CFLAGS. A hack in ui/sdl.c can be
> removed after this change.
>
> WINVER is still needed for all compilations with MinGW, so it cannot be
> defined in the new file. Replace its numeric value by a symbolic value to
> show which API is requested.
[...]
> diff --git a/block/raw-win32.c b/block/raw-win32.c
> index ae1c8e6..95b27a5 100644
> --- a/block/raw-win32.c
> +++ b/block/raw-win32.c
> @@ -23,13 +23,13 @@
>   */
>  #include "qemu-common.h"
>  #include "qemu/timer.h"
> +#include "qemu/winapi.h"        /* HANDLE (in raw-aio.h) */

Such comments get out of date real fast.  I treat them as noise.

If raw-aio.h needs stuff from winapi.h, why doesn't raw-aio.h include
it?

>  #include "block/block_int.h"
>  #include "qemu/module.h"
>  #include "raw-aio.h"
>  #include "trace.h"
>  #include "block/thread-pool.h"
>  #include "qemu/iov.h"
> -#include <windows.h>
>  #include <winioctl.h>
>  
>  #define FTYPE_FILE 0
[...]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/5] w32: Replace Windows specific data types in common header files
  2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 4/5] w32: Replace Windows specific data types in common header files Stefan Weil
@ 2014-03-10 15:17   ` Stefan Hajnoczi
  2014-03-10 18:34     ` Stefan Weil
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Hajnoczi @ 2014-03-10 15:17 UTC (permalink / raw)
  To: Stefan Weil
  Cc: kwolf, peter.maydell, Anthony Liguori, qemu-devel, stefanha,
	pbonzini, Andreas Färber

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 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/5] w32: Reduce dependency on Windows API
  2014-03-07 22:17 [Qemu-devel] [PATCH v2 0/5] w32: Reduce dependency on Windows API Stefan Weil
                   ` (4 preceding siblings ...)
  2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 5/5] block: Review include statements for winioctl.h Stefan Weil
@ 2014-03-10 15:18 ` Stefan Hajnoczi
  5 siblings, 0 replies; 13+ messages in thread
From: Stefan Hajnoczi @ 2014-03-10 15:18 UTC (permalink / raw)
  To: Stefan Weil; +Cc: kwolf, pbonzini, qemu-devel, stefanha, peter.maydell

On Fri, Mar 07, 2014 at 11:17:42PM +0100, Stefan Weil wrote:
> The first 4 patches reduce the number of files which depend on
> windows.h by about 90 percent.
> 
> This reduces the compilation time, allows
> removing some hacks and avoids name space pollution.
> 
> Patch 5 is optional and new here.
> 
> Changes in v2:
> 
> * Change name of new include file include/qemu/winapi.h
>   (suggested by Paolo Bonzini)
> 
> * Don't replace Win32 data types in block/raw-aio.h
>   (suggested by Kevin Wolf)
> 
> * Replace Win32 data types in central header files by
>   new QEMU data types instead of basic C data types
>   (suggested by Peter Maydell and Stefan Hajnoczi)
> 
> [PATCH v2 1/5] w32: Add and use intermediate include file for windows.h
> [PATCH v2 2/5] w32: Move inline function from header file to C source
> [PATCH v2 3/5] w32: Reduce dependencies in sysemu/os-win32.h
> [PATCH v2 4/5] w32: Replace Windows specific data types in common
> [PATCH v2 5/5] block: Review include statements for winioctl.h

Personally, I think the code was nicer without adding this indirection
to avoid longer compile times.  But then you're one of the few people
who cares about Windows, so if it makes your life easier that's a good
thing.

Stefan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/5] w32: Add and use intermediate include file for windows.h
  2014-03-10  8:56   ` Markus Armbruster
@ 2014-03-10 17:38     ` Stefan Weil
  0 siblings, 0 replies; 13+ messages in thread
From: Stefan Weil @ 2014-03-10 17:38 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: kwolf, peter.maydell, Anthony Liguori, Jan Kiszka, qemu-devel,
	Gerd Hoffmann, stefanha, pbonzini

Am 10.03.2014 09:56, schrieb Markus Armbruster:
> Stefan Weil <sw@weilnetz.de> writes:
> 
>> Including windows.h from the new file include/qemu/winapi.h allows
>> better tracking of the files which depend on the Windows API.
>>
>> 1864 *.o files depend on windows.h in a typical build, only 88 *.o files
>> don't.
>>
>> The windows.h specific macro WIN32_LEAN_AND_MEAN is now defined in the new
>> file and no longer part of the QEMU_CFLAGS. A hack in ui/sdl.c can be
>> removed after this change.
>>
>> WINVER is still needed for all compilations with MinGW, so it cannot be
>> defined in the new file. Replace its numeric value by a symbolic value to
>> show which API is requested.
> [...]
>> diff --git a/block/raw-win32.c b/block/raw-win32.c
>> index ae1c8e6..95b27a5 100644
>> --- a/block/raw-win32.c
>> +++ b/block/raw-win32.c
>> @@ -23,13 +23,13 @@
>>   */
>>  #include "qemu-common.h"
>>  #include "qemu/timer.h"
>> +#include "qemu/winapi.h"        /* HANDLE (in raw-aio.h) */
> 
> Such comments get out of date real fast.  I treat them as noise.

I agree that dependencies change over time. That's the main reason for
this kind of comments: it is easy to check whether the comment still
applies. If not, chances are good that the include statement is no
longer needed and can be removed together with the comment. How can
human reviewers see quickly why an include statement is needed when
there is no comment? Usually they can see that only for the most common
cases, and for all others they need help from the compiler.

> 
> If raw-aio.h needs stuff from winapi.h, why doesn't raw-aio.h include
> it?

It could, of course, but I assume that there is no special reason why it
does not. A lot of QEMU sources don't include everything which they need
explicitly. I thought about moving the include statement from
block/raw-win32.c and block/win32-aio.c to raw-aio.h, but decided to
minimize my changes and keep the current structure.

Stefan

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/5] w32: Replace Windows specific data types in common header files
  2014-03-10 15:17   ` Stefan Hajnoczi
@ 2014-03-10 18:34     ` Stefan Weil
  2014-03-11  7:51       ` Stefan Hajnoczi
  2014-03-11  8:06       ` Paolo Bonzini
  0 siblings, 2 replies; 13+ messages in thread
From: Stefan Weil @ 2014-03-10 18:34 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: kwolf, peter.maydell, Anthony Liguori, qemu-devel, stefanha,
	pbonzini, Andreas Färber

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/5] w32: Replace Windows specific data types in common header files
  2014-03-10 18:34     ` Stefan Weil
@ 2014-03-11  7:51       ` Stefan Hajnoczi
  2014-03-11  8:06       ` Paolo Bonzini
  1 sibling, 0 replies; 13+ messages in thread
From: Stefan Hajnoczi @ 2014-03-11  7:51 UTC (permalink / raw)
  To: Stefan Weil
  Cc: Kevin Wolf, Peter Maydell, Anthony Liguori, qemu-devel,
	Stefan Hajnoczi, Paolo Bonzini, Andreas Färber

On Mon, Mar 10, 2014 at 7:34 PM, Stefan Weil <sw@weilnetz.de> wrote:
> 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.

I don't see a hard reason against merging this series.  There are
trade-offs but you have thought through the alternatives.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

On Mon, Mar 10, 2014 at 7:34 PM, Stefan Weil <sw@weilnetz.de> wrote:
> 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
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/5] w32: Replace Windows specific data types in common header files
  2014-03-10 18:34     ` Stefan Weil
  2014-03-11  7:51       ` Stefan Hajnoczi
@ 2014-03-11  8:06       ` Paolo Bonzini
  1 sibling, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2014-03-11  8:06 UTC (permalink / raw)
  To: Stefan Weil, Stefan Hajnoczi
  Cc: kwolf, peter.maydell, Anthony Liguori, qemu-devel, stefanha,
	Andreas Färber

Il 10/03/2014 19:34, Stefan Weil ha scritto:
> 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.

Any solution is fine.  I don't like particularly this series, but I 
don't dislike it enough to override your judgement as Win32 maintainer.

I agree with Markus about not liking the comments on the #include lines, 
though.

Paolo

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-03-11  8:06 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-07 22:17 [Qemu-devel] [PATCH v2 0/5] w32: Reduce dependency on Windows API Stefan Weil
2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 1/5] w32: Add and use intermediate include file for windows.h Stefan Weil
2014-03-10  8:56   ` Markus Armbruster
2014-03-10 17:38     ` Stefan Weil
2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 2/5] w32: Move inline function from header file to C source Stefan Weil
2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 3/5] w32: Reduce dependencies in sysemu/os-win32.h Stefan Weil
2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 4/5] w32: Replace Windows specific data types in common header files Stefan Weil
2014-03-10 15:17   ` Stefan Hajnoczi
2014-03-10 18:34     ` Stefan Weil
2014-03-11  7:51       ` Stefan Hajnoczi
2014-03-11  8:06       ` Paolo Bonzini
2014-03-07 22:17 ` [Qemu-devel] [PATCH v2 5/5] block: Review include statements for winioctl.h Stefan Weil
2014-03-10 15:18 ` [Qemu-devel] [PATCH v2 0/5] w32: Reduce dependency on Windows API Stefan Hajnoczi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).