-
-
Notifications
You must be signed in to change notification settings - Fork 3k
add macOS handling for totalSystemMemory #24903
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
base: master
Are you sure you want to change the base?
Conversation
41a5088 to
9492937
Compare
This doesn't matter too much since I'll get to it when I do #6389 anyway. But yes, it probably should.
I think just remove the dead |
469c895 to
243ced2
Compare
| @@ -1762,7 +1762,19 @@ pub fn totalSystemMemory() TotalSystemMemoryError!u64 { | |||
| error.NameTooLong, error.UnknownName => unreachable, | |||
| else => return error.UnknownTotalSystemMemory, | |||
| }; | |||
| return @as(usize, @intCast(physmem)); | |||
| return @as(u64, @intCast(physmem)); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type of physmem should be changed to u64, making the cast unnecessary.
Hi, I just started learing zig. I was trying to fetch total system memory on macOS and noticed that
totalSystemMemorydoes not support it.This PR adds it.
I believe there are a few issues with
totalSystemMemoryfor.freebsdandstd.posix.sysctlbynameZ, but I do not know what the correct approach to take heretotalSystemMemoryfor.freebsdreturnsusizeinstead ofu64, I believe this should be fixedtotalSystemMemoryfor.freebsddoes not handle all the errors fromstd.posix.sysctlbynameZ.std.posix.sysctlbynameZnever returnsNameTooLong. So either we should change the return type, or, as @andrewrk did in 3640303 we should addname_lenhandling. I did not find any mention ofENAMETOOLONGin manpages for freebsd/posix. So I am having a hard time understanding which way is correct here.Just for the context I know that there were discussions in #16350 that resulted in the rollback of patches that already implemented this functionality before. Please let me know what should be done to meet contributing standards to avoid same fate as previous contributor 🙂
I am ready to provide patches for 1 and 2 in this PR or create separate one
Thanks!