Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

curl_threads: fix MSVC compiler warning #1717

Closed
wants to merge 1 commit into from

Conversation

MarcelRaad
Copy link
Member

@MarcelRaad MarcelRaad commented Aug 1, 2017

Use LongToHandle to convert from long to HANDLE in the Win32
implementation.
This should fix the following warning when compiling with
MSVC 11 (2012) in 64-bit mode visible in #1711:

lib\curl_threads.c(113): warning C4306: 'type cast' : conversion from 'long' to 'HANDLE' of greater size

LongToHandle is available in the February 2003 Platform SDK, so I think we should be safe. Alternatively, INVALID_HANDLE_VALUE could be used (the documented value in the _beginthreadex documentation is -1L, though).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 75.355% when pulling 06a57ae on MarcelRaad:msvc11_c4306 into 53d137d on curl:master.

Use LongToHandle to convert from long to HANDLE in the Win32
implementation.
This should fix the following warning when compiling with
MSVC 11 (2012) in 64-bit mode:
lib\curl_threads.c(113): warning C4306:
'type cast' : conversion from 'long' to 'HANDLE' of greater size

Closes curl#1717
@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 75.321% when pulling 12b4fb7 on MarcelRaad:msvc11_c4306 into 821a085 on curl:master.

@MarcelRaad MarcelRaad closed this in 0139545 Aug 1, 2017
@MarcelRaad MarcelRaad deleted the msvc11_c4306 branch August 1, 2017 15:34
@jay
Copy link
Member

jay commented Aug 1, 2017

I can't reproduce this warning in VS 2010 in a test project at W4 or 2015 in curl project at W4. original mingw only defines LongToHandle if win64 so that will need to be addressed. from what i understand beginthreadex can return 0 or -1 uintptr_t on error (0 but it contradicts because -1 in some versions if startaddress is null). CreateThread can return 0 HANDLE on error. do you get a warning if you drop the L for example if((t == 0) || (t == (curl_thread_t)(uintptr_t)-1)) {

@jay
Copy link
Member

jay commented Aug 1, 2017

I tried in VS2010 curl project at W4 and I can reproduce it there, this fixes it:

diff --git a/lib/curl_threads.c b/lib/curl_threads.c
index 1989710..53fed9c 100644
--- a/lib/curl_threads.c
+++ b/lib/curl_threads.c
@@ -110,7 +110,7 @@ curl_thread_t Curl_thread_create(unsigned int (CURL_STDCALL *func) (void *),
 #else
   t = (curl_thread_t)_beginthreadex(NULL, 0, func, arg, 0, NULL);
 #endif
-  if((t == 0) || (t == (curl_thread_t)-1L)) {
+  if((t == 0) || (t == (curl_thread_t)(uintptr_t)-1)) {
 #ifdef _WIN32_WCE
     DWORD gle = GetLastError();
     errno = ((gle == ERROR_ACCESS_DENIED ||

@MarcelRaad
Copy link
Member Author

MarcelRaad commented Aug 1, 2017

Thanks @jay! That's strange as original MinGW cannot even compile for Win64.
Unfortunately older MSVC versions don't have uintptr_t, IIRC it was introduced in VS 2003. Maybe LONG_PTR can be used instead, that would have been my preferred alternative, but I decided for LongToHandle as they were introduced in the same SDK version. I'll check tomorrow.

@MarcelRaad
Copy link
Member Author

@jay I can compile successfully with MinGW and --enable-threaded-resolver --disable-pthreads. It looks like LongToHandle is only defined when not compiling for Win64, so it should be OK. I had to look twice too though, the code in basetsd.h looks like this:

#if defined(_WIN64)
#define __int3264   __int64
#define ADDRESS_TAG_BIT 0x40000000000UI64
#else /*  !_WIN64 */
#define __int3264   __int32
#define ADDRESS_TAG_BIT 0x80000000UL
#define HandleToUlong( h ) ((ULONG)(ULONG_PTR)(h) )
#define HandleToLong( h ) ((LONG)(LONG_PTR) (h) )
#define LongToHandle( h) ((HANDLE)(LONG_PTR) (h))
#define PtrToUlong( p ) ((ULONG)(ULONG_PTR) (p) )
#define PtrToLong( p ) ((LONG)(LONG_PTR) (p) )
#define PtrToUint( p ) ((UINT)(UINT_PTR) (p) )
#define PtrToInt( p ) ((INT)(INT_PTR) (p) )
#define PtrToUshort( p ) ((unsigned short)(ULONG_PTR)(p) )
#define PtrToShort( p ) ((short)(LONG_PTR)(p) )
#define IntToPtr( i )    ((VOID*)(INT_PTR)((int)i))
#define UIntToPtr( ui )  ((VOID*)(UINT_PTR)((unsigned int)ui))
#define LongToPtr( l )   ((VOID*)(LONG_PTR)((long)l))
#define ULongToPtr( ul )  ((VOID*)(ULONG_PTR)((unsigned long)ul))
#endif /* !_WIN64 */

@jay
Copy link
Member

jay commented Aug 3, 2017

It looks like LongToHandle is only defined when not compiling for Win64, so it should be OK. I had to look twice too though, the code in basetsd.h looks like this:

You're right, I had misinterpreted that, thanks for following up.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants