Skip to content

Define ssize_t as intptr_t in Windows #1298

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

Merged
merged 1 commit into from
May 3, 2025

Conversation

feihongmeilian
Copy link
Contributor

After checking, the modification typedef intptr_t ssize_t; is more suitable as it self-adapts to the platform and won't cause conflicts when combined with asynchronous libuv under Win32.

Copy link
Collaborator

@michael-grunder michael-grunder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have or use a Windows machine but from some quick reading it appears this is more correct.

WIN32 -> intptr_t -> __int32
WIN64 -> intptr_t -> __int64

Whereas long long is 64 bits on WIN32.

@feihongmeilian
Copy link
Contributor Author

feihongmeilian commented Apr 23, 2025

Yes, according to my research, the C99 standard specifies that long long occupies at least 8 bytes on 32-bit machines.
However, on ​Win32, ssize_t should inherently occupy ​4 bytes, whereas defining it as long long would force it to ​8 bytes, which violates the semantic meaning of ssize_t. Additionally, this discrepancy could cause conflicts when integrating ​libuv​ for asynchronous operations. Below is libuv's definition for reference:https://github.com/libuv/libuv/blob/v1.x/include/uv/win.h#L26

@feihongmeilian
Copy link
Contributor Author

This problem surfaced when I was compiling redis-plus-plus. I hope my program can run on Win64, Win32, and Linux. However, on Win32, due to the different definitions of ssize_t in hiredis and libuv, redis-plus-plus fails to compile.
Meanwhile, the information I've looked up shows that libuv's definition better conforms to the semantics of ssize_t, so I submitted this PR.

@michael-grunder
Copy link
Collaborator

Sorry if it wasn't clear, I mean this PR seems correct 😄

@michael-grunder
Copy link
Collaborator

cc @uglide Let me know if you have any issues. Seems good to me.

@michael-grunder michael-grunder merged commit 6859cc7 into redis:master May 3, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants