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

[Core] Underlying Memory Arena uses size_t but Clay_CreateArenaWithCapacityAndMemory expects a uint32_t argument #291

Open
cugone opened this issue Mar 2, 2025 · 4 comments
Labels
Attempted Fix A fix has been implemented but not yet confirmed by the issue reporter. bug Something isn't working

Comments

@cugone
Copy link

cugone commented Mar 2, 2025

See Title. When building in x64 this will fail without a cast and is potentially error-prone. Changing the argument in Clay_CreateArenaWithCapacityAndMemory to a size_t will allow the compiler to use the correct version of size_t and the cast will no longer be needed.

@nicbarker
Copy link
Owner

Hello, and great catch! Will get this fixed up immediately.

@nicbarker nicbarker changed the title Underlying Memory Arena uses size_t but Clay_CreateArenaWithCapacityAndMemory expects a uint32_t argument [Core] Underlying Memory Arena uses size_t but Clay_CreateArenaWithCapacityAndMemory expects a uint32_t argument Mar 2, 2025
@nicbarker
Copy link
Owner

Fixed here: 5571c00

@nicbarker nicbarker added bug Something isn't working Attempted Fix A fix has been implemented but not yet confirmed by the issue reporter. labels Mar 2, 2025
@cugone
Copy link
Author

cugone commented Mar 3, 2025

Similarly Clay_MinMemorySize(void) returns a uint32_t but slipping a smaller type into a larger type is okay, it's just me being pedantic.

@Nelarius
Copy link

Nelarius commented Mar 8, 2025

@cugone Noticed the mismatched return type for Clay_MinMemorySize as well, submitted a pr which just explicitly casts the value for now: #317

Agree that using size_t would be more consistent, but this enables compilation with a bunch of warnings enabled for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Attempted Fix A fix has been implemented but not yet confirmed by the issue reporter. bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants