[ Return to Bugs & Features | Post Text | Post File | Prev | Next ]

STR #3271

Trouble Report Files:

	Name/Time/Date	Filename/Size
	#1  szukw000 05:55 Dec 27, 2015	Socket-r10957.dif 5k
	#2  szukw000 08:40 Dec 28, 2015	socket.txt 3k

Trouble Report Comments:

	Name/Time/Date	Text
	#1  szukw000 05:55 Dec 27, 2015	winsock2.h: typedef UINT_PTR        SOCKET; winfried
	#2  AlbrechtS 03:05 Dec 28, 2015	https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751%28v=vs.85%29.aspx#UINT_PTR <quote> This type is declared in BaseTsd.h as follows: #if defined(_WIN64)  typedef unsigned __int64 UINT_PTR; #else  typedef unsigned int UINT_PTR; #endif </quote>
	#3  AlbrechtS 03:26 Dec 28, 2015	Thanks for the report, and thanks for the patch. Please tell us why you think that we should apply your patch, i.e. compiler warnings/messages, the platform and compiler and so on. On a first glance I can see that MS defines SOCKET as unsigned, whereas the Linux definition is int. This makes writing portable code difficult, and the FLTK API uses int for add_fd() etc.. If we were going to change that I believe that we would break existing programs - at least we'd introduce compiler warnings in existing programs that use (currently appropriate) casts. Maybe, or not... Also, there is a comment in the code your patch proposes to change: // Keep avoiding having the socket deps at that level but mke sure it will work in both 32 & 64 bit builds So the author (I didn't check who that was) did intentionally not include winsock2.h in FL/Fl.H, and we should probably not change this. According to the MSDN docs cited above the only real difference to the effect of your patch (in FL/Fl.H) would be the (un-)signedness of FL_SOCKET in the /32-Bit/ Windows case. I assume that the existing code intended to keep the WIN32 case as it was before (int, as on all other platforms anyway), but added the _WIN64 case for some reason - probably compiler warnings. Conclusion: The current API and ABI uses "int" in all cases, the proposed patch would break the ABI at least on 64-Bit Windows, hence this is not going to be applicable in FLTK 1.3.x. I don't know if it would be wise to do such an API change in FLTK 1.4 though. I tend to not doing it (leave the API using "int") although Windows users might have to live with casts. Comments, anybody?
	#4  szukw000 08:49 Dec 28, 2015	The file 'socket.txt' contains the answers to comment #3. One could avoid to include winsock2.h, but: MS-WINDOWS on 32-Bit machine: 'int' is 32-Bits wide, 'unsigned int' is 32-Bits wide. MS-WINDOWS on 64-Bit machine: 'int' is 32-Bits wide, 'unsigned int' is 32-Bits wide. So if you cast a 64-Bits word to 32-Bits, you obviously will loose data. winfried
	#5  AlbrechtS 16:13 Sep 01, 2016	Bumped version from 1.3 to 1.4-feature. Changed priority from 3 (moderate) to 4 (high) for FLTK 1.4. We know that the definition of FL_SOCKET for Windows, particularly 64-bit versions, is critical, but we can't change the API/ABI in FLTK 1.3.x. I hope that we can find a better solution in FLTK 1.4, hence the version bump.
	#6  szukw000 22:04 Sep 01, 2016	Supposed the definitions of MS-WINDOWS and of LINUX/CYGWIN do not change: #if defined(WIN32) && !defined(__CYGWIN__) # if defined(_WIN64) #  define FL_SOCKET unsigned __int64 # else #  define FL_SOCKET unsigned int # endif #else # define FL_SOCKET int #endif It would not be necessary to include 'winsock2.h': -  static void add_fd(int fd, Fl_FD_Handler cb, void* = 0); +  static void add_fd(FL_SOCKET fd, Fl_FD_Handler cb, void* = 0); etc. winfried
	#7  ianmacarthur 14:49 Feb 13, 2018	For 1.4, while ABI is still in flux, this typedef is possibly our best choice here?
	#8  ianmacarthur 14:52 Feb 13, 2018	For 1.3.x - maybe we are stuck with hoping that no one uses a socket with a number that is "too large"... Which is *probably* OK on most practical implementations, AFAIK. I'm not entirely clear why MS decided to make this 64-bit, TBH, when there are so many other places in their API's where they probably should have gone 64-bit and decided not too. It seems an odd choice here...
	#9  AlbrechtS 13:15 Jan 16, 2023	Consolidating STR 3271 (this STR) and STR 3357 (https://www.fltk.org/str.php?L3357). The latter is a duplicate of this one and will be closed. Regarding further progress, please refer also to STR 3357, but work and discuss exclusively here (STR 3271). Note: I'm evaluating the issue and the provided patches, but this doesn't mean that this can be solved.

[ Return to Bugs & Features | Post Text | Post File ]