threads / patch / 41198

patch, 4 partsmingw: avoid warnings when casting HANDLEs to int

Subject: [PATCH 2/4] mingw: avoid warnings when casting HANDLEs to int

## tl;dr

7 messages between Jan 15, 2016 and Jan 16, 2016. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Johannes Schindelin· Jan 15, 2016, 13:24 UTC · lore

[PATCH 0/4] Fix compile warnings with MSys2's compiler

This is the second batch of Windows-specific patches from the Git for Windows project. Naturally, it is based on the first batch of patches that fixed the compile errors.

Please note that this series does the *minimal* job of fixing the compile warnings. Don't try to run the tests, they will still fail. You have been warned.

Johannes Schindelin (4):
  mingw: avoid redefining S_* constants
  mingw: avoid warnings when casting HANDLEs to int
  mingw: squash another warning about a cast
  mingw: uglify (a, 0) definitions to shut up warnings
 compat/mingw.c              | 9 ++++++---
 compat/mingw.h              | 4 ++++
 compat/nedmalloc/malloc.c.h | 3 ++-
 compat/poll/poll.c          | 2 +-
 compat/win32/pthread.h      | 7 +++++--
 compat/winansi.c            | 3 ++-
 6 files changed, 20 insertions(+), 8 deletions(-)
-- 
2.7.0.windows.1.7.g55a05c8
Johannes Schindelin· Jan 15, 2016, 13:24 UTC · re: Johannes Schindelin · lore

[PATCH 1/4] mingw: avoid redefining S_* constants

When compiling with MSys2's compiler, these constants are already defined.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/mingw.h | 4 ++++
 1 file changed, 4 insertions(+)
Show changes to compat/mingw.h +4 −0
diff --git a/compat/mingw.h b/compat/mingw.h
index b3e5044..3a404ff 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -14,14 +14,18 @@ typedef int socklen_t;
 #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)
 #define S_ISSOCK(x) 0
 
+#ifndef S_IRWXG
 #define S_IRGRP 0
 #define S_IWGRP 0
 #define S_IXGRP 0
 #define S_IRWXG (S_IRGRP | S_IWGRP | S_IXGRP)
+#endif
+#ifndef S_IRWXO
 #define S_IROTH 0
 #define S_IWOTH 0
 #define S_IXOTH 0
 #define S_IRWXO (S_IROTH | S_IWOTH | S_IXOTH)
+#endif
 
 #define S_ISUID 0004000
 #define S_ISGID 0002000

-- 
2.7.0.windows.1.7.g55a05c8
Johannes Schindelin· Jan 15, 2016, 13:24 UTC · re: Johannes Schindelin · lore

HANDLE is defined internally as a void *, but in many cases it is actually guaranteed to be a 32-bit integer. In these cases, GCC should not warn about a cast of a pointer to an integer of a different type because we know exactly what we are doing.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/mingw.c     | 9 ++++++---
 compat/poll/poll.c | 2 +-
 compat/winansi.c   | 3 ++-
 3 files changed, 9 insertions(+), 5 deletions(-)
Show changes to 3 files +9 −5

compat/mingw.c, compat/poll/poll.c, compat/winansi.c

diff --git a/compat/mingw.c b/compat/mingw.c
index 1b3530a..8437c9a 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -6,6 +6,8 @@
 #include "../run-command.h"
 #include "../cache.h"
 
+#define HCAST(type, handle) ((type)(intptr_t)handle)
+
 static const int delay[] = { 0, 1, 10, 20, 40 };
 
 int err_win_to_posix(DWORD winerr)
@@ -691,13 +693,13 @@ int pipe(int filedes[2])
 		errno = err_win_to_posix(GetLastError());
 		return -1;
 	}
-	filedes[0] = _open_osfhandle((int)h[0], O_NOINHERIT);
+	filedes[0] = _open_osfhandle(HCAST(int, h[0]), O_NOINHERIT);
 	if (filedes[0] < 0) {
 		CloseHandle(h[0]);
 		CloseHandle(h[1]);
 		return -1;
 	}
-	filedes[1] = _open_osfhandle((int)h[1], O_NOINHERIT);
+	filedes[1] = _open_osfhandle(HCAST(int, h[1]), O_NOINHERIT);
 	if (filedes[1] < 0) {
 		close(filedes[0]);
 		CloseHandle(h[1]);
@@ -1846,7 +1848,8 @@ void mingw_open_html(const char *unixpath)
 		die("cannot run browser");
 
 	printf("Launching default browser to display HTML ...\n");
-	r = (int)ShellExecute(NULL, "open", htmlpath, NULL, "\\", SW_SHOWNORMAL);
+	r = HCAST(int, ShellExecute(NULL, "open", htmlpath,
+				NULL, "\\", SW_SHOWNORMAL));
 	FreeLibrary(shell32);
 	/* see the MSDN documentation referring to the result codes here */
 	if (r <= 32) {
diff --git a/compat/poll/poll.c b/compat/poll/poll.c
index db4e03e..b10adc7 100644
--- a/compat/poll/poll.c
+++ b/compat/poll/poll.c
@@ -76,7 +76,7 @@
 
 #ifdef WIN32_NATIVE
 
-#define IsConsoleHandle(h) (((long) (h) & 3) == 3)
+#define IsConsoleHandle(h) (((long) (intptr_t) (h) & 3) == 3)
 
 static BOOL
 IsSocketHandle (HANDLE h)
diff --git a/compat/winansi.c b/compat/winansi.c
index ceff55b..4549848 100644
--- a/compat/winansi.c
+++ b/compat/winansi.c
@@ -452,7 +452,8 @@ static HANDLE duplicate_handle(HANDLE hnd)
 	HANDLE hresult, hproc = GetCurrentProcess();
 	if (!DuplicateHandle(hproc, hnd, hproc, &hresult, 0, TRUE,
 			DUPLICATE_SAME_ACCESS))
-		die_lasterr("DuplicateHandle(%li) failed", (long) hnd);
+		die_lasterr("DuplicateHandle(%li) failed",
+			(long) (intptr_t) hnd);
 	return hresult;
 }
 

-- 
2.7.0.windows.1.7.g55a05c8
Johannes Schindelin· Jan 15, 2016, 13:24 UTC · re: Johannes Schindelin · lore

[PATCH 3/4] mingw: squash another warning about a cast

MSys2's compiler is correct that casting a "void *" to a "DWORD" loses precision, but in the case of pthread_exit() we know that the value fits into a DWORD.

Just like casting handles to DWORDs, let's work around this issue by casting to "intrptr_t" first, and immediately cast to the final type.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/win32/pthread.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to compat/win32/pthread.h +1 −1
diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
index 8ad1873..d3dd872 100644
--- a/compat/win32/pthread.h
+++ b/compat/win32/pthread.h
@@ -77,7 +77,7 @@ extern pthread_t pthread_self(void);
 
 static inline int pthread_exit(void *ret)
 {
-	ExitThread((DWORD)ret);
+	ExitThread((DWORD)(intptr_t)ret);
 }
 
 typedef DWORD pthread_key_t;

-- 
2.7.0.windows.1.7.g55a05c8
Johannes Schindelin· Jan 15, 2016, 13:24 UTC · re: Johannes Schindelin · lore

[PATCH 4/4] mingw: uglify (a, 0) definitions to shut up warnings

When the result of a (a, 0) expression is not used, MSys2's GCC version finds it necessary to complain with a warning:

	right-hand operand of comma expression has no effect

Let's just pretend to use the 0 value and have a peaceful and quiet life again.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/nedmalloc/malloc.c.h | 3 ++-
 compat/win32/pthread.h      | 5 ++++-
 2 files changed, 6 insertions(+), 2 deletions(-)
Show changes to 2 files +6 −2

compat/nedmalloc/malloc.c.h, compat/win32/pthread.h

diff --git a/compat/nedmalloc/malloc.c.h b/compat/nedmalloc/malloc.c.h
index f216a2a..ac0b7b4 100644
--- a/compat/nedmalloc/malloc.c.h
+++ b/compat/nedmalloc/malloc.c.h
@@ -1798,9 +1798,10 @@ struct win32_mlock_t
   volatile long threadid;
 };
 
+static inline int return_0(int i) { return 0; }
 #define MLOCK_T               struct win32_mlock_t
 #define CURRENT_THREAD        win32_getcurrentthreadid()
-#define INITIAL_LOCK(sl)      (memset(sl, 0, sizeof(MLOCK_T)), 0)
+#define INITIAL_LOCK(sl)      (memset(sl, 0, sizeof(MLOCK_T)), return_0(0))
 #define ACQUIRE_LOCK(sl)      win32_acquire_lock(sl)
 #define RELEASE_LOCK(sl)      win32_release_lock(sl)
 #define TRY_LOCK(sl)          win32_try_lock(sl)
diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h
index d3dd872..20b35a2 100644
--- a/compat/win32/pthread.h
+++ b/compat/win32/pthread.h
@@ -18,7 +18,10 @@
  */
 #define pthread_mutex_t CRITICAL_SECTION
 
-#define pthread_mutex_init(a,b) (InitializeCriticalSection((a)), 0)
+static inline int return_0(int i) {
+	return 0;
+}
+#define pthread_mutex_init(a,b) return_0((InitializeCriticalSection((a)), 0))
 #define pthread_mutex_destroy(a) DeleteCriticalSection((a))
 #define pthread_mutex_lock EnterCriticalSection
 #define pthread_mutex_unlock LeaveCriticalSection

-- 
2.7.0.windows.1.7.g55a05c8
Johannes Schindelin· Jan 16, 2016, 17:00 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/4] Fix compile warnings with MSys2's compiler

Hi Junio,
On Fri, 15 Jan 2016, Junio C Hamano wrote:
> Thanks, queued.
Thank you!
> The last one does look ugly as you said; I do not think of a better
> alternative to offer, though X-<.

It makes me happy that I did not miss any obvious way to make this look better, and it makes me sad you cannot find a better alternative either ;-)

Ciao, Dscho

← back to recent threads