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

Add (void *) cast directly to GetProcAddress calls. #103354

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Feb 27, 2025

No description provided.

Comment on lines -86 to -89
#if defined(__GNUC__)
// Workaround GCC warning from -Wcast-function-type.
#define GetProcAddress (void *)GetProcAddress
#endif
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it wouldn't be better to put this in typedefs.h so it's always available.
Since it compiles fine on MSVC without (void *) it might be easy to miss in future additions.

Copy link
Member Author

Choose a reason for hiding this comment

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

GCC build in CI should always catch feature additions (and I guess clang-cl as well from now), and I do not think all Windows specific files include typedefs.h, so not sure if it's better. Also, some parts of the code were alredy using direct (void *) cast.

@akien-mga akien-mga merged commit bb88938 into godotengine:master Feb 27, 2025
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@Drako
Copy link
Contributor

Drako commented Mar 1, 2025

From a language standard perspective there is no guarantee that code pointers and data pointers have the same size.
So while dlsym returns a data pointer (which technically shouldn't be cast to a code pointer), GetProcAddress returns a code pointer (which technically shouldn't be cast to a data pointer).

It just so happens that on all major OS used on the common x86, x64, arm and arm64 architectures they do have the same size, so this does work. But if we wanted to be nitpicky, this is undefined behavior.

By the way this is the reason why you have the cast in the first place, because GCC warns about exactly this.
The correct way would be to use the FARPROC type or its definition void (*)() instead of void*.

@bruvzg
Copy link
Member Author

bruvzg commented Mar 1, 2025

The correct way would be to use the FARPROC type or its definition void (*)() instead of void*.

FARPROC definition is long long int (*)(), and that's what warning is about (cast from 'FARPROC' to 'X' converts to incompatible function type).

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.

3 participants