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.

Sign up for GitHub

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

Jump to bottom

Alias ssize_t to SSIZE_T only for MSVC (fix for mingw) #4708

Merged
merged 1 commit into from
Apr 19, 2021
Merged

Alias ssize_t to SSIZE_T only for MSVC (fix for mingw) #4708

merged 1 commit into from
Apr 19, 2021

Conversation

giordano
Copy link
Contributor

@giordano giordano commented Apr 17, 2021 •
edited
Loading

MinGW defines ssize_t as int for a 32-bit target, which conflicts with the definition of SSIZE_T, which is LONG_PTR, i.e. long for a 32-bit target.

@giordano giordano mentioned this pull request Apr 17, 2021
Merged
@giordano
Copy link
Contributor Author

giordano commented Apr 17, 2021 •
edited
Loading

Note that this fix is similar to #2027, but separating ssize_t for MSVC was undone in e3c9340 (part of #2354)

@kunaltyagi
Copy link
Member

@giordano Could you please resolve the conflicts (by rebasing or merging)?

@giordano
Alias ssize_t to SSIZE_T only for MSVC
69f6c51 
MinGW defines `ssize_t` as `int` for a 32-bit target
(https://github.com/mirror/mingw-w64/blob/c6e13e0c105eab7797c2373819b49fff6b05566c/mingw-w64-headers/crt/time.h#L76-L80),
which conflicts with the definition of `SSIZE_T`, which is `LONG_PTR`,
i.e. `long` for a 32-bit
target (https://docs.microsoft.com/en-us/windows/win32/winprog/windows-data-types).
@giordano
Copy link
Contributor Author

Done! I've also added a comment in the code to make it clearer why this is needed, and avoid someone will remove it again in the future

@kunaltyagi
Copy link
Member

Thanks @giordano! Do you have links to how PCL is being used in Julia?

PS: This is a pure MinGW change, right? We don't officially support MinGW, so you might have to keep an eye out yourself for this. Our CI will not catch errors of this type

@kunaltyagi kunaltyagi changed the title Alias ssize_t to SSIZE_T only for MSVC Alias ssize_t to SSIZE_T only for MSVC (fix for mingw) Apr 19, 2021
@kunaltyagi kunaltyagi merged commit 12261af into PointCloudLibrary:master Apr 19, 2021
@giordano giordano deleted the mg/ssize_t branch April 19, 2021 08:25
@giordano
Copy link
Contributor Author

Thanks @giordano! Do you have links to how PCL is being used in Julia?

I don't, and I'm not sure it's used as of yet, but probably @Crghilardi knows more.

PS: This is a pure MinGW change, right? We don't officially support MinGW, so you might have to keep an eye out yourself for this. Our CI will not catch errors of this type

Ok, luckily it wasn't needed much work to get it working. We try to be good downstream by upstreaming all relevant patches 🙂 BTW, last patch for this round is at #4707

@Crghilardi
Copy link

@kunaltyagi I don't have anything to show right now, this was a first step in getting some of this functionality wrapped in Julia. Will try to remember to send a note when something more concrete is ready. Thank you for the responsiveness for all the PRs, this has been a pleasant experience

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kunaltyagi kunaltyagi kunaltyagi approved these changes

@larshg larshg larshg approved these changes

Assignees
No one assigned
Labels
module: io platform: windows
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@giordano @kunaltyagi @Crghilardi @larshg