fpagliughi / sockpp

Modern C++ socket library.
BSD 3-Clause "New" or "Revised" License
782 stars 126 forks source link

Definition of ssize_t is wrong #39

Closed snej closed 4 years ago

snej commented 4 years ago

sockpp has its own declaration of ssize_t in platform.h, but on non-Windows systems it defines it as int. This is wrong on 64-bit OSs that use 32-bit ints, which includes all Apple platforms. (Dunno about Linux or BSD.) This leads to compile errors if a source file ends up including both platform.h and the OS's declaration. That aside, there are potentially actual integer overflow bugs if dealing with sizes > 4GB.

https://github.com/fpagliughi/sockpp/blob/180f18d55b46fb6e8b6723a8dacc8f3733f1a47d/include/sockpp/platform.h#L78-L86

You shouldn't need to declare this at all. Instead:

(Also, it's kind of icky that the types declared in platform.h are all in the global namespace. Could they be put in the sockpp namespace like the rest of the API?)

snej commented 4 years ago

Aw crap, I didn't notice that all that stuff is wrapped in #if defined(_WIN32).

@borrrden tells me that the definition on Windows should still be fixed to use the one from the system, because the system one ends up being typedef'd to long not int, which is incompatible.

fpagliughi commented 4 years ago

Ha. I saw this and thought, "That must be something new". But I find references to it going back to 2002, so I must have missed something.

Sure, I'd much rather use definitions from each system than make them up myself. Just wish Microsoft was more aligned to the rest of the world.

fpagliughi commented 4 years ago

OK. I'll cherry-pick a1271fb, and a few other non-SSL commits in #17, and include then in an upcoming point release.

fpagliughi commented 4 years ago

Done with a1271fb.