[Qemu-devel] [PATCH 0/9] w32: Reduce dependency on Windows API

[Qemu-devel] [PATCH 0/9] w32: Reduce dependency on Windows API

[Stefan Weil]

Today, most .o files depend on windows.h when QEMU is built with MinGW or
MinGW-w64 (32 and 64 bit builds). These patches reduce the number of such
files from more than 1800 to less than 180. They also allow removing some
hacks which were needed because of conflicts with windows.h. A nasty
compiler warning is fixed, too.

The modifications were tested with MinGW (32 bit native) and MinGW-w64
(32 and 64 bit cross).

Regards
Stefan W.

[PATCH 1/9] util/iov: Use qemu/sockets.h instead of conditional code
[PATCH 2/9] exec: Remove unneeded include files
[PATCH 3/9] qemu-img: Remove unneeded include files
[PATCH 4/9] qga: Remove unneeded include file
[PATCH 5/9] vl: Remove unneeded include file
[PATCH 6/9] w32: Add and use intermediate include file for windows.h
[PATCH 7/9] w32: Move inline function from header file to C source
[PATCH 8/9] w32: Reduce dependencies in sysemu/os-win32.h
[PATCH 9/9] w32: Replace Windows specific data types in common

[Qemu-devel] [PATCH 1/9] util/iov: Use qemu/sockets.h instead of conditional code

[Stefan Weil]

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 util/iov.c |    9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/util/iov.c b/util/iov.c
index bb46c04..3996300 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -17,14 +17,7 @@
  */
 
 #include "qemu/iov.h"
-
-#ifdef _WIN32
-# include <windows.h>
-# include <winsock2.h>
-#else
-# include <sys/types.h>
-# include <sys/socket.h>
-#endif
+#include "qemu/sockets.h"
 
 size_t iov_from_buf(const struct iovec *iov, unsigned int iov_cnt,
                     size_t offset, const void *buf, size_t bytes)
-- 
1.7.10.4

[Qemu-devel] [PATCH 2/9] exec: Remove unneeded include files

[Stefan Weil]

This file does not depend on windows.h.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 exec.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/exec.c b/exec.c
index b69fd29..680268a 100644
--- a/exec.c
+++ b/exec.c
@@ -17,9 +17,7 @@
  * License along with this library; if not, see <http://www.gnu.org/licenses/>.
  */
 #include "config.h"
-#ifdef _WIN32
-#include <windows.h>
-#else
+#ifndef _WIN32
 #include <sys/types.h>
 #include <sys/mman.h>
 #endif
-- 
1.7.10.4

[Qemu-devel] [PATCH 3/9] qemu-img: Remove unneeded include files

[Stefan Weil]

There is no dependency on windows.h, and the standard include files are
already included by qemu-common.h.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 qemu-img.c |    6 ------
 1 file changed, 6 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 0927b09..fbd4b47 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -32,12 +32,6 @@
 #include "block/block_int.h"
 #include "block/qapi.h"
 #include <getopt.h>
-#include <stdio.h>
-#include <stdarg.h>
-
-#ifdef _WIN32
-#include <windows.h>
-#endif
 
 typedef struct img_cmd_t {
     const char *name;
-- 
1.7.10.4

[Qemu-devel] [PATCH 4/9] qga: Remove unneeded include file

[Stefan Weil]

This file does not depend on windows.h.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 qga/main.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/qga/main.c b/qga/main.c
index c58b26a..6157deb 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -35,7 +35,6 @@
 #ifdef _WIN32
 #include "qga/service-win32.h"
 #include "qga/vss-win32.h"
-#include <windows.h>
 #endif
 #ifdef __linux__
 #include <linux/fs.h>
-- 
1.7.10.4

[Qemu-devel] [PATCH 5/9] vl: Remove unneeded include file

[Stefan Weil]

This file does not depend on windows.h.

Cc: Anthony Liguori <aliguori@amazon.com>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 vl.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/vl.c b/vl.c
index 9379d33..c178504 100644
--- a/vl.c
+++ b/vl.c
@@ -38,10 +38,6 @@
 #include <libvdeplug.h>
 #endif
 
-#ifdef _WIN32
-#include <windows.h>
-#endif
-
 #ifdef CONFIG_SDL
 #if defined(__APPLE__) || defined(main)
 #include <SDL.h>
-- 
1.7.10.4

[Qemu-devel] [PATCH 6/9] w32: Add and use intermediate include file for windows.h

[Stefan Weil]

Including windows.h from the new file include/sysemu/os-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>
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/sysemu/os-win32.h     |    2 +-
 include/sysemu/os-winapi.h    |   22 ++++++++++++++++++++++
 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/sysemu/os-winapi.h

diff --git a/audio/audio_win_int.c b/audio/audio_win_int.c
index e132405..eeb41bb 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 "sysemu/os-winapi.h"
 #include <mmsystem.h>
 
 #include "audio.h"
diff --git a/audio/dsoundaudio.c b/audio/dsoundaudio.c
index e2d89fd..0c0b928 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 "sysemu/os-winapi.h"
 #include <mmsystem.h>
 #include <objbase.h>
 #include <dsound.h>
diff --git a/audio/winwaveaudio.c b/audio/winwaveaudio.c
index 8dbd145..1d8a87d 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 "sysemu/os-winapi.h"
 #include <mmsystem.h>
 
 #include "audio_win_int.h"
diff --git a/block.c b/block.c
index 6f4baca..c2f3891 100644
--- a/block.c
+++ b/block.c
@@ -47,7 +47,7 @@
 #endif
 
 #ifdef _WIN32
-#include <windows.h>
+#include "sysemu/os-winapi.h"
 #endif
 
 struct BdrvDirtyBitmap {
diff --git a/block/raw-win32.c b/block/raw-win32.c
index ae1c8e6..44d76f3 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -29,7 +29,7 @@
 #include "trace.h"
 #include "block/thread-pool.h"
 #include "qemu/iov.h"
-#include <windows.h>
+#include "sysemu/os-winapi.h"
 #include <winioctl.h>
 
 #define FTYPE_FILE 0
diff --git a/block/win32-aio.c b/block/win32-aio.c
index 5d1d199..e8b1d6c 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -29,7 +29,7 @@
 #include "raw-aio.h"
 #include "qemu/event_notifier.h"
 #include "qemu/iov.h"
-#include <windows.h>
+#include "sysemu/os-winapi.h"
 #include <winioctl.h>
 
 #define FTYPE_FILE 0
diff --git a/configure b/configure
index 39f2a1a..a460cf0 100755
--- a/configure
+++ b/configure
@@ -610,7 +610,7 @@ fi
 
 if test "$mingw32" = "yes" ; then
   EXESUF=".exe"
-  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..1b75f29 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 "sysemu/os-winapi.h"
 #endif
 
 struct EventNotifier {
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 45588d7..f82ad17 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 "sysemu/os-winapi.h"
 #include <winsock2.h>
 #include <ws2tcpip.h>
 
diff --git a/include/qemu/thread-win32.h b/include/qemu/thread-win32.h
index 3d58081..127a7ba 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 "sysemu/os-winapi.h"
 
 struct QemuMutex {
     CRITICAL_SECTION lock;
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index bf8523a..2dd3b1e 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 "sysemu/os-winapi.h"
 #include <winsock2.h>
 
 /* Workaround for older versions of MinGW. */
diff --git a/include/sysemu/os-winapi.h b/include/sysemu/os-winapi.h
new file mode 100644
index 0000000..a7680fd
--- /dev/null
+++ b/include/sysemu/os-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 OS_WINAPI_H
+#define OS_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 /* OS_WINAPI_H */
diff --git a/net/tap-win32.c b/net/tap-win32.c
index 91e9e84..d55088c 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 "sysemu/os-winapi.h"
 #include <winioctl.h>
 
 //=============
diff --git a/os-win32.c b/os-win32.c
index 50b7f6f..8fcaf3f 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 "sysemu/os-winapi.h"
 #include <mmsystem.h>
 #include <unistd.h>
 #include <fcntl.h>
diff --git a/qga/channel-win32.c b/qga/channel-win32.c
index 8a303f3..3a0bf13 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 "sysemu/os-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 50094dd..a57b644 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 "sysemu/os-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..d8eaccd 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 "sysemu/os-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..718ff29 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 "sysemu/os-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..3be8b96 100644
--- a/qga/vss-win32.c
+++ b/qga/vss-win32.c
@@ -11,7 +11,7 @@
  */
 
 #include <stdio.h>
-#include <windows.h>
+#include "sysemu/os-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..9ec5daa 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 "sysemu/os-winapi.h"
 #include <shlwapi.h>
 
 /* Reduce warnings to include vss.h */
diff --git a/slirp/slirp.h b/slirp/slirp.h
index e4a1bd4..6aad613 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -9,7 +9,7 @@
 
 typedef char *caddr_t;
 
-# include <windows.h>
+# include "sysemu/os-winapi.h"
 # include <winsock2.h>
 # include <ws2tcpip.h>
 # include <sys/timeb.h>
diff --git a/translate-all.c b/translate-all.c
index 1ac0246..69110d1 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 "sysemu/os-winapi.h"
 #else
 #include <sys/types.h>
 #include <sys/mman.h>
diff --git a/ui/sdl.c b/ui/sdl.c
index 9d8583c..7c9ca8b 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>
 #include <SDL_syswm.h>
 
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 50be044..2ea5377 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 "sysemu/os-winapi.h"
 #include <glib.h>
 #include <stdlib.h>
 #include "config-host.h"
-- 
1.7.10.4

[Qemu-devel] [PATCH 7/9] w32: Move inline function from header file to C source

[Stefan Weil]

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..e39d28d 100644
--- a/util/qemu-timer-common.c
+++ b/util/qemu-timer-common.c
@@ -28,7 +28,16 @@
 
 #ifdef _WIN32
 
-int64_t clock_freq;
+#include "sysemu/os-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

[Qemu-devel] [PATCH 8/9] w32: Reduce dependencies in sysemu/os-win32.h

[Stefan Weil]

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..e480b29 100644
--- a/coroutine-win32.c
+++ b/coroutine-win32.c
@@ -24,6 +24,7 @@
 
 #include "qemu-common.h"
 #include "block/coroutine_int.h"
+#include "sysemu/os-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 2dd3b1e..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 "sysemu/os-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..92220e3 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 "sysemu/os-winapi.h" /* TODO: workaround, remove */
 #include "qemu-common.h"
 
 #ifdef CONFIG_VNC_PNG
-- 
1.7.10.4

[Qemu-devel] [PATCH 9/9] w32: Replace Windows specific data types in common header files

[Stefan Weil]

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 and LONG
can be directly replaced by void * and long. CRITICAL_SECTION
is replaced by a new struct of the same size.

Add an explicit dependency on sysemu/os-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: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 block/raw-aio.h               |    4 ++--
 cpus.c                        |    4 +++-
 include/qemu/event_notifier.h |    8 ++------
 include/qemu/main-loop.h      |    4 ++--
 include/qemu/thread-win32.h   |   29 ++++++++++++++++++++---------
 include/qom/cpu.h             |    2 +-
 ui/vnc-enc-tight.c            |    5 -----
 util/event_notifier-win32.c   |    1 +
 util/qemu-thread-win32.c      |   17 +++++++++--------
 9 files changed, 40 insertions(+), 34 deletions(-)

diff --git a/block/raw-aio.h b/block/raw-aio.h
index 7ad0a8a..fd3f3bf 100644
--- a/block/raw-aio.h
+++ b/block/raw-aio.h
@@ -42,9 +42,9 @@ BlockDriverAIOCB *laio_submit(BlockDriverState *bs, void *aio_ctx, int fd,
 #ifdef _WIN32
 typedef struct QEMUWin32AIOState QEMUWin32AIOState;
 QEMUWin32AIOState *win32_aio_init(void);
-int win32_aio_attach(QEMUWin32AIOState *aio, HANDLE hfile);
+int win32_aio_attach(QEMUWin32AIOState *aio, void *hfile);
 BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs,
-        QEMUWin32AIOState *aio, HANDLE hfile,
+        QEMUWin32AIOState *aio, void *hfile,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
         BlockDriverCompletionFunc *cb, void *opaque, int type);
 #endif
diff --git a/cpus.c b/cpus.c
index 945d85b..4e91e08 100644
--- a/cpus.c
+++ b/cpus.c
@@ -39,7 +39,9 @@
 #include "qemu/bitmap.h"
 #include "qemu/seqlock.h"
 
-#ifndef _WIN32
+#ifdef _WIN32
+#include "sysemu/os-winapi.h" /* SuspendThread, ... */
+#else
 #include "qemu/compatfd.h"
 #endif
 
diff --git a/include/qemu/event_notifier.h b/include/qemu/event_notifier.h
index 1b75f29..222ef8c 100644
--- a/include/qemu/event_notifier.h
+++ b/include/qemu/event_notifier.h
@@ -15,13 +15,9 @@
 
 #include "qemu-common.h"
 
-#ifdef _WIN32
-#include "sysemu/os-winapi.h"
-#endif
-
 struct EventNotifier {
 #ifdef _WIN32
-    HANDLE event;
+    void *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 *);
+void *event_notifier_get_handle(EventNotifier *);
 #endif
 
 #endif
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 6f0200a..68d2798 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(void *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(void *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 127a7ba..d2ff862 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 "sysemu/os-winapi.h"
+
+/* QemuCriticalSection is a substitute for CRITICAL_SECTION and
+ * introduced here to avoid dependencies on windows.h. */
+
+typedef struct {
+    void *DebugInfo;
+    long LockCount;
+    long RecursionCount;
+    void *OwningThread;
+    void *LockSemaphore;
+    unsigned long *SpinCount;
+} QemuCriticalSection;
 
 struct QemuMutex {
-    CRITICAL_SECTION lock;
-    LONG owner;
+    QemuCriticalSection lock;
+    long owner;
 };
 
 struct QemuCond {
-    LONG waiters, target;
-    HANDLE sema;
-    HANDLE continue_event;
+    long waiters, target;
+    void *sema;
+    void *continue_event;
 };
 
 struct QemuSemaphore {
-    HANDLE sema;
+    void *sema;
 };
 
 struct QemuEvent {
-    HANDLE event;
+    void *event;
 };
 
 typedef struct QemuThreadData QemuThreadData;
@@ -28,6 +39,6 @@ struct QemuThread {
 };
 
 /* Only valid for joinable threads.  */
-HANDLE qemu_thread_get_handle(QemuThread *thread);
+void *qemu_thread_get_handle(QemuThread *thread);
 
 #endif
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 367eda1..6ea48c4 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -170,7 +170,7 @@ struct CPUState {
 
     struct QemuThread *thread;
 #ifdef _WIN32
-    HANDLE hThread;
+    void *hThread;
 #endif
     int thread_id;
     uint32_t host_tid;
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 92220e3..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 "sysemu/os-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..bf39268 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 "sysemu/os-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..6d1f777 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 "sysemu/os-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

Re: [Qemu-devel] [PATCH 8/9] w32: Reduce dependencies in sysemu/os-win32.h

[Max Filippov]

On Sun, Feb 23, 2014 at 9:02 PM, Stefan Weil <sw@weilnetz.de> wrote:
> 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(-)

For xtensa part: Acked-by: Max Filippov <jcmvbkbc@gmail.com>

--
Thanks.
-- Max

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

[Kevin Wolf]

Am 23.02.2014 um 18:02 hat Stefan Weil geschrieben:
> 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 and LONG
> can be directly replaced by void * and long. CRITICAL_SECTION
> is replaced by a new struct of the same size.
> 
> Add an explicit dependency on sysemu/os-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: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>

Not sure if I understand the problem. Why is it bad to depend on
windows.h?

Kevin

Re: [Qemu-devel] [PATCH 6/9] w32: Add and use intermediate include file for windows.h

[Paolo Bonzini]

Il 23/02/2014 18:02, Stefan Weil ha scritto:
> diff --git a/qga/channel-win32.c b/qga/channel-win32.c
> index 8a303f3..3a0bf13 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 "sysemu/os-winapi.h"
>  #include <errno.h>
>  #include <io.h>
>  #include "qga/guest-agent-core.h"

qga/ is not supposed to include include/sysemu (which is just for 
qemu-system-*).  I guess you can call the new file include/qemu/winapi.h

Paolo

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

[Andreas Färber]

Am 23.02.2014 18:02, schrieb Stefan Weil:
> 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 and LONG
> can be directly replaced by void * and long. CRITICAL_SECTION
> is replaced by a new struct of the same size.
> 
> Add an explicit dependency on sysemu/os-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: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
[...]
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 367eda1..6ea48c4 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h

First of all, why was I not CC'ed on this change?
File is listed under "CPU" subsystem in MAINTAINERS.

Patches not getting sufficient review is one risk, causing merge
conflicts another. People should be aware of changes you make in their
files, even if "just" Windows-related.

> @@ -170,7 +170,7 @@ struct CPUState {
>  
>      struct QemuThread *thread;
>  #ifdef _WIN32
> -    HANDLE hThread;
> +    void *hThread;
>  #endif
>      int thread_id;
>      uint32_t host_tid;

I had moved the field unchanged from another header, I believe. I don't
think this is a good change (assuming the Windows API is still using
this type and not void*), especially since it's #ifdef'ed anyway.

And I don't understand why reducing the number of files dependent on
windows.h is a good thing either. Your cover letter does not explain.
We've not been trying to reduce dependencies on glib either, nor is
there precedence for replacing other pointer types elsewhere (e.g.
qemu_irq). Wherever possible, we've been using the most precise pointer
type to let the compiler or static analysis tools help us catch mismatches.

Is all this just to speed up rebuilds after MinGW updates?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

[Peter Maydell]

On 24 February 2014 10:17, Kevin Wolf <kwolf@redhat.com> wrote:
> Not sure if I understand the problem. Why is it bad to depend on
> windows.h?

It drags in an enormous pile of stuff that grabs a lot of
namespace that it would be nice to not have to avoid by
fiddling with QEMU code. (For instance the Spitz board
clashes with a MOD_SHIFT #define.)

thanks
-- PMM

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

[Kevin Wolf]

Am 24.02.2014 um 14:24 hat Peter Maydell geschrieben:
> On 24 February 2014 10:17, Kevin Wolf <kwolf@redhat.com> wrote:
> > Not sure if I understand the problem. Why is it bad to depend on
> > windows.h?
> 
> It drags in an enormous pile of stuff that grabs a lot of
> namespace that it would be nice to not have to avoid by
> fiddling with QEMU code. (For instance the Spitz board
> clashes with a MOD_SHIFT #define.)

Right, I can understand that you want to avoid device models or other
random source files to have a polluted namespace.

The file that I'm CCed for, however, is block/raw-aio.h, which is only
included in the raw-posix block driver (there it's #ifdef'ed away) and
in the raw-win32 one (which has an obvious dependency on Windows
headers).

Can't see how dropping the type safety improves anything there. I might
see the improvement of not polluting the namespace in other places, but
am still not convinced that using void* instead of specific types is a
good idea.

Kevin

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

[Peter Maydell]

On 24 February 2014 13:44, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 24.02.2014 um 14:24 hat Peter Maydell geschrieben:
>> On 24 February 2014 10:17, Kevin Wolf <kwolf@redhat.com> wrote:
>> > Not sure if I understand the problem. Why is it bad to depend on
>> > windows.h?
>>
>> It drags in an enormous pile of stuff that grabs a lot of
>> namespace that it would be nice to not have to avoid by
>> fiddling with QEMU code. (For instance the Spitz board
>> clashes with a MOD_SHIFT #define.)
>
> Right, I can understand that you want to avoid device models or other
> random source files to have a polluted namespace.
>
> The file that I'm CCed for, however, is block/raw-aio.h, which is only
> included in the raw-posix block driver (there it's #ifdef'ed away) and
> in the raw-win32 one (which has an obvious dependency on Windows
> headers).

Yes, there doesn't seem too much point in that particular case.

> Can't see how dropping the type safety improves anything there. I might
> see the improvement of not polluting the namespace in other places, but
> am still not convinced that using void* instead of specific types is a
> good idea.

We might be able to abstract away the differences into a
QEMU-defined type in some cases rather than using void*.
(I haven't looked to see if this is feasible.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 6/9] w32: Add and use intermediate include file for windows.h

[Stefan Weil]

Am 24.02.2014 11:26, schrieb Paolo Bonzini:
> Il 23/02/2014 18:02, Stefan Weil ha scritto:
>> diff --git a/qga/channel-win32.c b/qga/channel-win32.c
>> index 8a303f3..3a0bf13 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 "sysemu/os-winapi.h"
>>  #include <errno.h>
>>  #include <io.h>
>>  #include "qga/guest-agent-core.h"
> 
> qga/ is not supposed to include include/sysemu (which is just for
> qemu-system-*).  I guess you can call the new file include/qemu/winapi.h
> 
> Paolo

Ah, thanks for explaining. I'll use the suggested name in an updated
series. But you are aware that qga/ already includes sysemu/os-posix.h
and sysemu/os-win32.h indirectly? Should those two files also be in
include/qemu?

Stefan

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

[Stefan Weil]

Am 24.02.2014 14:07, schrieb Andreas Färber:
> Am 23.02.2014 18:02, schrieb Stefan Weil:
>> These header files are used by most QEMU source files. If they
>> depend on windows.h, all those source files do so, too.
>>
[...]
> 
> First of all, why was I not CC'ed on this change?
> File is listed under "CPU" subsystem in MAINTAINERS.
> 
> Patches not getting sufficient review is one risk, causing merge
> conflicts another. People should be aware of changes you make in their
> files, even if "just" Windows-related.

Sorry, I simply missed your name in the lengthy output from
get_maintainer.pl.

> I had moved the field unchanged from another header, I believe. I don't
> think this is a good change (assuming the Windows API is still using
> this type and not void*), especially since it's #ifdef'ed anyway.

Please see my mail to Kevin (which will be sent in a moment).

> 
> And I don't understand why reducing the number of files dependent on
> windows.h is a good thing either. Your cover letter does not explain.
> We've not been trying to reduce dependencies on glib either, nor is
> there precedence for replacing other pointer types elsewhere (e.g.
> qemu_irq). Wherever possible, we've been using the most precise pointer
> type to let the compiler or static analysis tools help us catch mismatches.

See also my other mail.

> 
> Is all this just to speed up rebuilds after MinGW updates?

The .d files don't track dependencies to system include files. A MinGW
update won't trigger new compilations, so this is not a reason at all.

Regards
Stefan

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

[Stefan Weil]

Am 24.02.2014 14:51, schrieb Peter Maydell:
> On 24 February 2014 13:44, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 24.02.2014 um 14:24 hat Peter Maydell geschrieben:
>>> On 24 February 2014 10:17, Kevin Wolf <kwolf@redhat.com> wrote:
>>>> Not sure if I understand the problem. Why is it bad to depend on
>>>> windows.h?
>>>
>>> It drags in an enormous pile of stuff that grabs a lot of
>>> namespace that it would be nice to not have to avoid by
>>> fiddling with QEMU code. (For instance the Spitz board
>>> clashes with a MOD_SHIFT #define.)
>>
>> Right, I can understand that you want to avoid device models or other
>> random source files to have a polluted namespace.
>>
>> The file that I'm CCed for, however, is block/raw-aio.h, which is only
>> included in the raw-posix block driver (there it's #ifdef'ed away) and
>> in the raw-win32 one (which has an obvious dependency on Windows
>> headers).
> 
> Yes, there doesn't seem too much point in that particular case.
> 
>> Can't see how dropping the type safety improves anything there. I might
>> see the improvement of not polluting the namespace in other places, but
>> am still not convinced that using void* instead of specific types is a
>> good idea.
> 
> We might be able to abstract away the differences into a
> QEMU-defined type in some cases rather than using void*.
> (I haven't looked to see if this is feasible.)
> 
> thanks
> -- PMM
> 

Kevin, I'll have a look on that special case in v2 of my patch series.
I'll say more on type safety and Peter's suggestion in another mail.

Stefan

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

[Stefan Weil]

Am 24.02.2014 11:17, schrieb Kevin Wolf:
> Am 23.02.2014 um 18:02 hat Stefan Weil geschrieben:
>> 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 and LONG
>> can be directly replaced by void * and long. CRITICAL_SECTION
>> is replaced by a new struct of the same size.
>>
>> Add an explicit dependency on sysemu/os-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: Kevin Wolf <kwolf@redhat.com>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> Not sure if I understand the problem. Why is it bad to depend on
> windows.h?
>
> Kevin

It's not generally bad, but there are some disadvantages if too many
files depend on it.

Let me explain first what windows.h is (I assume you know that, but
maybe others don't): it is the central Windows API header which includes
lots of other files, so we don't talk about a single header file but a
rather large collection of such files. Another header file which QEMU
uses very often is glib.h for the glib-2.0 API. I counted the number of
files which get included by a simple #include of one of these two header
files and also measured their size:

glib.h (Debian wheezy)
  564910 bytes in 86 files, this includes 22 standard header files like
stddef.h, ...

windows.h (Debian wheezy MinGW-w64)
  3659593 bytes in 116 files (macro WIN32_LEAN_AND_MEAN is not defined),
10 of them are standard header files
  1147623 bytes in 46 files (macro WIN32_LEAN_AND_MEAN is defined)

QEMU uses WIN32_LEAN_AND_MEAN, but even then windows.h gets about 1 MB
of header file code whereas glib.h gets more files but only 0.5 MB of
header file code.

This header file code is analysed by the compiler more than 1800 times
for a full build, so the compiler reads and parses about 2 GB of code.
Native builds on Windows are slow. Reading and parsing 2 GB does not
make them faster. I don't know whether caching compilers are available
for Windows and if so whether they work good.

There exist several implementations of the header files for the Windows
API: the original header files from Microsoft (which exist in different
versions, but QEMU does not use them), the MinGW implementation (was
standard for QEMU), the MinGW-w64 implementation (now recommended for
QEMU, supports 32 and 64 bit) and the Cygwin implementation (currently
unsupported and not working with QEMU). All of them differ and need
special handling. By including windows.h we get potential problems and
namespace pollutions . The current QEMU code already contains some hacks
for such problems (which are removed by my patches) and would need one
more hack (which is not needed with my patches). See also a discussion
which I and Peter had nearly a year ago:
http://patchwork.ozlabs.org/patch/244758/. It was the trigger for this
patch series.

All those different implementations of the Windows API header files use
the same definitions for basic types as HANDLE or LONG, so we don't
loose anything by replacing the Windows specific types by standard C
types. Nor would we gain anything if we introduced new types like
QEMU_HANDLE or QEMU_LONG. Everybody is used to files and sockets being
int on UNIX, so why not use void pointers for Windows handles in this
special case if it avoids pulling gigabytes of code?

Regards
Stefan

Re: [Qemu-devel] [PATCH 0/9] w32: Reduce dependency on Windows API

[Stefan Weil]

Am 23.02.2014 18:02, schrieb Stefan Weil:
> Today, most .o files depend on windows.h when QEMU is built with MinGW or
> MinGW-w64 (32 and 64 bit builds). These patches reduce the number of such
> files from more than 1800 to less than 180. They also allow removing some
> hacks which were needed because of conflicts with windows.h. A nasty
> compiler warning is fixed, too.
> 
> The modifications were tested with MinGW (32 bit native) and MinGW-w64
> (32 and 64 bit cross).
> 
> Regards
> Stefan W.
> 
> [PATCH 1/9] util/iov: Use qemu/sockets.h instead of conditional code
> [PATCH 2/9] exec: Remove unneeded include files
> [PATCH 3/9] qemu-img: Remove unneeded include files
> [PATCH 4/9] qga: Remove unneeded include file
> [PATCH 5/9] vl: Remove unneeded include file
> [PATCH 6/9] w32: Add and use intermediate include file for windows.h
> [PATCH 7/9] w32: Move inline function from header file to C source
> [PATCH 8/9] w32: Reduce dependencies in sysemu/os-win32.h
> [PATCH 9/9] w32: Replace Windows specific data types in common
> 

This patch series needs at least one more iteration with a v2.

Patches 1-5 can be applied independently of the rest and are trivial, so
maybe they can be applied by qemu-trivial, or I can send a pull request
for only those patches as soon as they were reviewed. That would reduce
the size of my v2 patch set.

Thanks
Stefan

Re: [Qemu-devel] [PATCH 6/9] w32: Add and use intermediate include file for windows.h

[Paolo Bonzini]

Il 25/02/2014 00:00, Stefan Weil ha scritto:
> Ah, thanks for explaining. I'll use the suggested name in an updated
> series. But you are aware that qga/ already includes sysemu/os-posix.h
> and sysemu/os-win32.h indirectly? Should those two files also be in
> include/qemu?

No, but perhaps part of it should be moved to qemu/osdep.h?

Paolo

Re: [Qemu-devel] [PATCH 0/9] w32: Reduce dependency on Windows API

[Paolo Bonzini]

Il 25/02/2014 06:32, Stefan Weil ha scritto:
> Am 23.02.2014 18:02, schrieb Stefan Weil:
>> Today, most .o files depend on windows.h when QEMU is built with MinGW or
>> MinGW-w64 (32 and 64 bit builds). These patches reduce the number of such
>> files from more than 1800 to less than 180. They also allow removing some
>> hacks which were needed because of conflicts with windows.h. A nasty
>> compiler warning is fixed, too.
>>
>> The modifications were tested with MinGW (32 bit native) and MinGW-w64
>> (32 and 64 bit cross).
>>
>> Regards
>> Stefan W.
>>
>> [PATCH 1/9] util/iov: Use qemu/sockets.h instead of conditional code
>> [PATCH 2/9] exec: Remove unneeded include files
>> [PATCH 3/9] qemu-img: Remove unneeded include files
>> [PATCH 4/9] qga: Remove unneeded include file
>> [PATCH 5/9] vl: Remove unneeded include file
>> [PATCH 6/9] w32: Add and use intermediate include file for windows.h
>> [PATCH 7/9] w32: Move inline function from header file to C source
>> [PATCH 8/9] w32: Reduce dependencies in sysemu/os-win32.h
>> [PATCH 9/9] w32: Replace Windows specific data types in common
>>
>
> This patch series needs at least one more iteration with a v2.
>
> Patches 1-5 can be applied independently of the rest and are trivial, so
> maybe they can be applied by qemu-trivial, or I can send a pull request
> for only those patches as soon as they were reviewed. That would reduce
> the size of my v2 patch set.

Patches 1-5 are

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks for all the effort you put in janitorial work, it's as thankless 
as it's useful!

Paolo

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

[Andreas Färber]

Am 25.02.2014 00:07, schrieb Stefan Weil:
> Am 24.02.2014 14:07, schrieb Andreas Färber:
>> Am 23.02.2014 18:02, schrieb Stefan Weil:
>>> These header files are used by most QEMU source files. If they
>>> depend on windows.h, all those source files do so, too.
>>>
> [...]
>>
>> First of all, why was I not CC'ed on this change?
>> File is listed under "CPU" subsystem in MAINTAINERS.
>>
>> Patches not getting sufficient review is one risk, causing merge
>> conflicts another. People should be aware of changes you make in their
>> files, even if "just" Windows-related.
> 
> Sorry, I simply missed your name in the lengthy output from
> get_maintainer.pl.
> 
>> I had moved the field unchanged from another header, I believe. I don't
>> think this is a good change (assuming the Windows API is still using
>> this type and not void*), especially since it's #ifdef'ed anyway.
> 
> Please see my mail to Kevin (which will be sent in a moment).

My opinion still stands. You can easily add "typedef void * HANDLE;" to
qemu-typedefs.h or osdep.h to retain telling type names, in this case
explaining why the variable is called "hThread" - h for "HANDLE".

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 0/9] w32: Reduce dependency on Windows API

[Andreas Färber]

Am 25.02.2014 09:20, schrieb Paolo Bonzini:
> Il 25/02/2014 06:32, Stefan Weil ha scritto:
>> Am 23.02.2014 18:02, schrieb Stefan Weil:
>>> Today, most .o files depend on windows.h when QEMU is built with
>>> MinGW or
>>> MinGW-w64 (32 and 64 bit builds). These patches reduce the number of
>>> such
>>> files from more than 1800 to less than 180. They also allow removing
>>> some
>>> hacks which were needed because of conflicts with windows.h. A nasty
>>> compiler warning is fixed, too.
>>>
>>> The modifications were tested with MinGW (32 bit native) and MinGW-w64
>>> (32 and 64 bit cross).
>>>
>>> Regards
>>> Stefan W.
>>>
>>> [PATCH 1/9] util/iov: Use qemu/sockets.h instead of conditional code
>>> [PATCH 2/9] exec: Remove unneeded include files
>>> [PATCH 3/9] qemu-img: Remove unneeded include files
>>> [PATCH 4/9] qga: Remove unneeded include file
>>> [PATCH 5/9] vl: Remove unneeded include file
>>> [PATCH 6/9] w32: Add and use intermediate include file for windows.h
>>> [PATCH 7/9] w32: Move inline function from header file to C source
>>> [PATCH 8/9] w32: Reduce dependencies in sysemu/os-win32.h
>>> [PATCH 9/9] w32: Replace Windows specific data types in common
>>>
>>
>> This patch series needs at least one more iteration with a v2.
>>
>> Patches 1-5 can be applied independently of the rest and are trivial, so
>> maybe they can be applied by qemu-trivial, or I can send a pull request
>> for only those patches as soon as they were reviewed. That would reduce
>> the size of my v2 patch set.
> 
> Patches 1-5 are
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Seconded, no objections to those,

Reviewed-by: Andreas Färber <afaerber@suse.de>

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

[Stefan Weil]

Am 25.02.2014 09:37, schrieb Andreas Färber:
> Am 25.02.2014 00:07, schrieb Stefan Weil:
[...]
>>> I had moved the field unchanged from another header, I believe. I don't
>>> think this is a good change (assuming the Windows API is still using
>>> this type and not void*), especially since it's #ifdef'ed anyway.
>> Please see my mail to Kevin (which will be sent in a moment).
> My opinion still stands. You can easily add "typedef void * HANDLE;" to
> qemu-typedefs.h or osdep.h to retain telling type names, in this case
> explaining why the variable is called "hThread" - h for "HANDLE".
>
> Regards,
> Andreas

I'm not sure whether I fully understand what you say.

Yes, adding a typedef for HANDLE or LONG is easy. I decided against that
solution because it is nearly impossible to avoid conflicts with the
typedefs pulled by windows.h. The QEMU typedef for HANDLE would have to
be protected by conditional compilation, and windows.h would have to be
included before that code. I am afraid that there will be lots of build
breakages in the future simply because people usually don't test with
MinGW and we have no simple rules how include statements should be ordered.

Stefan

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

[Stefan Hajnoczi]

On Sun, Feb 23, 2014 at 06:02:15PM +0100, Stefan Weil wrote:
> 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 and LONG
> can be directly replaced by void * and long. CRITICAL_SECTION
> is replaced by a new struct of the same size.

Can you use a typedef?  When I read code that uses HANDLE, I immediately
know this is Windows and which sorts of APIs can accept this HANDLE
value.

Now when I see void * it's just an opaque C type.  We've lost
information :(.

/* Since windows.h drags in a lot of headers, we define equivalent
 * typedefs for some core Win32 types.
 */
typedef void *QEMU_WIN32_HANDLE; /* is there a more concise name? */
...

Stefan

Re: [Qemu-devel] [PATCH 0/9] w32: Reduce dependency on Windows API

[Michael Tokarev]

23.02.2014 21:02, Stefan Weil wrote:
> Today, most .o files depend on windows.h when QEMU is built with MinGW or
> MinGW-w64 (32 and 64 bit builds). These patches reduce the number of such
> files from more than 1800 to less than 180. They also allow removing some
> hacks which were needed because of conflicts with windows.h. A nasty
> compiler warning is fixed, too.
> 
> The modifications were tested with MinGW (32 bit native) and MinGW-w64
> (32 and 64 bit cross).
> 
> Regards
> Stefan W.
> 
> [PATCH 1/9] util/iov: Use qemu/sockets.h instead of conditional code
> [PATCH 2/9] exec: Remove unneeded include files
> [PATCH 3/9] qemu-img: Remove unneeded include files
> [PATCH 4/9] qga: Remove unneeded include file
> [PATCH 5/9] vl: Remove unneeded include file
> [PATCH 6/9] w32: Add and use intermediate include file for windows.h
> [PATCH 7/9] w32: Move inline function from header file to C source
> [PATCH 8/9] w32: Reduce dependencies in sysemu/os-win32.h
> [PATCH 9/9] w32: Replace Windows specific data types in common

Applied 1-5 to -trivial, thanks!

/mjt