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

Correct compilation warnings for 'int' casts with correct types #75

Conversation

huguesdpdn-aerospace
Copy link
Contributor

@huguesdpdn-aerospace huguesdpdn-aerospace commented Aug 14, 2024

Fix a potential error on some specific architectures (aka devices with sizeof(int) different of sizeof(ssize_t)) where a size_t or ssize_t are not actually a real int. When dealing (subtracting or adding values) with size_t with negative value, int you should not be use but rather its dedicated opposite version, aka the signed one: ssize_t

Also, when dealing with tellg, actually it returns a streampos (see C++ Reference | std::streampos ) which should not be incremented or decremented with any integer. Instead, a dedicated method should be called by using std::streamoff for that in the class (see C++ Reference | std::streamoff )

@aiksiongkoh
Copy link
Collaborator

Windows build failed. @huguesdpdn-aerospace please check. Thanks.

@huguesdpdn-aerospace huguesdpdn-aerospace force-pushed the compilation_warnings_int_size_t branch from 88e07d0 to 7087d59 Compare August 15, 2024 21:49
@huguesdpdn-aerospace
Copy link
Contributor Author

Indeed Windows do not recognise ssize_t and I don't have any computer running not under Windows so I added a pre-processor macro with a typedef since SSIZE_T exists on Windows but not on Unix. Would need you @aiksiongkoh to execute the workflow and check if it compiles.

@aiksiongkoh
Copy link
Collaborator

@huguesdpdn-aerospace Please see build failures due to typo. Thanks.

@aiksiongkoh
Copy link
Collaborator

@huguesdpdn-aerospace Please remove the extra || in line below
15 | #if defined(WIN32) || defined(_WIN32) || || defined(WIN32) || defined(NT)

@huguesdpdn-aerospace huguesdpdn-aerospace force-pushed the compilation_warnings_int_size_t branch from 7087d59 to ae21e2b Compare September 3, 2024 19:37
@huguesdpdn-aerospace
Copy link
Contributor Author

huguesdpdn-aerospace commented Sep 3, 2024

Hello @aiksiongkoh , sorry I was in vacation. In addition, it was a stupid and easy typo to fix, sorry :/

Can you please approve the test workflow ? Thank you

@huguesdpdn-aerospace huguesdpdn-aerospace force-pushed the compilation_warnings_int_size_t branch from ae21e2b to f6d0aad Compare September 8, 2024 13:16
@huguesdpdn-aerospace
Copy link
Contributor Author

huguesdpdn-aerospace commented Sep 8, 2024

Hi @aiksiongkoh , thanks for the workflow approval.

Unfortunately, couldn't test under Windows and it seems that the macro SSIZE_T was not defined because under Windows, for SSIZE_T , documentation specify that we need to include Windows Data Types

A copy of the declared data can be found here: Windows 10 SDK - baseTsd.h

Need a new approval to re-execute the compilation tests

@aiksiongkoh aiksiongkoh merged commit 91f7038 into Ondsel-Development:main Sep 8, 2024
4 checks passed
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

Successfully merging this pull request may close these issues.

2 participants