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

Definition of ssize_t is wrong #39

Closed
snej opened this issue Feb 7, 2020 · 4 comments
Closed

Definition of ssize_t is wrong #39

snej opened this issue Feb 7, 2020 · 4 comments

Comments

@snej
Copy link
Contributor

snej commented Feb 7, 2020

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.

#ifndef _SSIZE_T_DEFINED
#define _SSIZE_T_DEFINED
#undef ssize_t
#ifdef _WIN64
using ssize_t = int64_t;
#else
using ssize_t = int;
#endif // _WIN64
#endif // _SSIZE_T_DEFINED

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

  • On a POSIX-compliant OS, #include <sys/types.h>.
  • On Windows, #include <BaseTsd.h> and then using ssize_t = SSIZE_T;.

(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
Copy link
Contributor Author

snej commented Feb 7, 2020

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.

borrrden added a commit to couchbasedeps/sockpp that referenced this issue Feb 7, 2020
@fpagliughi
Copy link
Owner

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
Copy link
Owner

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

fpagliughi pushed a commit that referenced this issue Feb 16, 2020
@fpagliughi
Copy link
Owner

Done with a1271fb.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants