-
Notifications
You must be signed in to change notification settings - Fork 198
Fix checkptr in Mem.Get() on darwin #77
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
Conversation
Lets the tests run through on MacOS ARM by properly using Sysctl. Credit goes to elastic@2c63039
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.
@jorbaum - thank you for taking a stab it getting this working on macOS!
Seems like the windows build is failing - I re-ran it just in case there was a flake and still got a failure.
It would be nice to enable macos-latest in the test workflow:
Did so. |
|
Regarding the failing windows build:
However, I would not see this as part of this Pull Request. Also, it is difficult to test for me as I do not have a windows development machine. |
|
@jorbaum - it looks like the main branch is currently succeeding on Windows[1] so it seems like there is something in this PR which is causing the windows issue[2]. Perhaps you can use this PR (or another) to bring over the Win32 changes, and rely on the github worker tests to verify? Thanks, -aram [1] https://github.com/cloudfoundry/gosigar/actions/runs/17929963955/job/50984988021 |
This PR combines two critical fixes: 1. macOS Fix (from cloudfoundry#77): - Updates sigar_darwin.go to use unix.SysctlUint64() for reading sysctl values - Fixes checkptr violations in Go 1.22+ when accessing uint64 values 2. Windows Fix (from elastic#116): - Implements ProcArgs.Get() using Win32 API instead of returning ErrNotImplemented - Replaces WMI queries with direct ReadProcessMemory and NtQueryInformationProcess calls - Adds helper functions: GetUserProcessParams, ReadProcessUnicodeString, ByteSliceToStringSlice - Adds syscall bindings for ReadProcessMemory and GetTickCount64 Fixes rkoster/rubionic-workspace#38
Thx. I did not see that it worked two weeks ago. AFAICS:
I guess that this is the issue. This PR should not change any windows-related code as it only changes the darwin file.
Looks like @rubionic is already taking on this task. Thanks! @aramprice As a quick fix I could change |
...until -latest is supported again.
Did so. |
|
@jorbaum thank you for this contribution! |
Lets the tests run through on MacOS ARM by properly using Sysctl.
Credit goes to
elastic@2c63039