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

windows: fix size of ssize_t #57

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

mkmkme
Copy link
Contributor

@mkmkme mkmkme commented Jul 30, 2024

Previously, long long was used unconditionally to define ssize_t on Windows. This meant it was always a 64-bit value. However, typically the size type is a 32-bit value on 32-bit machines and 64-bit value on 64-bit machines.

This didn't cause any issues until it was used on 32-bit Windows build of libvalkey-py. CPython defines the ssize_t type as well as libvalkey, but it defines the size to be 32 bit on 32 bit machines which caused a compilation error.

This commit changes the typedef to be SSIZE_T on Windows. Hence, it will be aligned with everything else.

@mkmkme mkmkme marked this pull request as ready for review July 30, 2024 11:28
@mkmkme
Copy link
Contributor Author

mkmkme commented Jul 30, 2024

Code format checker as well as macOS and Centos build failures look unrelated

include/valkey/sds.h Outdated Show resolved Hide resolved
Previously, `long long` was used unconditionally to define `ssize_t` on
Windows. This meant it was always a 64-bit value. However, typically
the size type is a 32-bit value on 32-bit machines and 64-bit value on
64-bit machines.

This didn't cause any issues until it was used on 32-bit Windows build
of `libvalkey-py`. CPython defines the `ssize_t` type as well as
`libvalkey`, but it defines the size to be 32 bit on 32 bit machines
which caused a compilation error.

This commit changes the typedef to be `SSIZE_T` on Windows. Hence, it
will be aligned with everything else.

Signed-off-by: Mikhail Koviazin <[email protected]>
@mkmkme mkmkme force-pushed the mkmkme/windows-ssize-t-size branch from 838d51a to 892226c Compare July 30, 2024 13:07
@mkmkme
Copy link
Contributor Author

mkmkme commented Jul 31, 2024

I don't think I have merge rights. If you think it's good to go, please merge so I can update it in libvalkey-py

@bjosv bjosv merged commit 0635b07 into valkey-io:main Jul 31, 2024
41 of 43 checks passed
@mkmkme mkmkme deleted the mkmkme/windows-ssize-t-size branch July 31, 2024 13:21
@bjosv bjosv added the enhancement New feature or request label Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants