Skip to content

Conversation

@aramprice
Copy link
Member

Based on #78

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces WMI queries with direct Windows API calls for retrieving process information, specifically implementing process argument retrieval functionality. The change modernizes the codebase by using native Windows system calls instead of WMI queries, which typically provides better performance and reduces external dependencies.

Key changes:

  • Added syscall wrappers for ReadProcessMemory and GetTickCount64 Windows APIs
  • Implemented utility functions to read process memory structures and parse Unicode strings
  • Replaced ErrNotImplemented placeholder with actual implementation in ProcArgs.Get()

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
sys/windows/zsyscall_windows.go Added low-level syscall wrappers for ReadProcessMemory and GetTickCount64 Windows APIs
sys/windows/syscall_windows.go Implemented helper functions for reading process memory structures, parsing Unicode strings, and processing command-line arguments
sigar_windows.go Replaced stub implementation of ProcArgs.Get() with working code that retrieves process arguments using direct API calls

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@aramprice aramprice requested a review from rkoster October 23, 2025 16:02
@aramprice aramprice force-pushed the update-windows-syscalls branch 4 times, most recently from 1d35da9 to baea763 Compare October 23, 2025 22:51
@rkoster rkoster moved this from Inbox to Waiting for Changes | Open for Contribution in Foundational Infrastructure Working Group Oct 30, 2025
@aramprice aramprice force-pushed the update-windows-syscalls branch from baea763 to 61786ef Compare October 30, 2025 15:58
@github-project-automation github-project-automation bot moved this from Waiting for Changes | Open for Contribution to Pending Merge | Prioritized in Foundational Infrastructure Working Group Oct 30, 2025
@rkoster rkoster merged commit 47674d5 into master Oct 30, 2025
13 checks passed
@github-project-automation github-project-automation bot moved this from Pending Merge | Prioritized to Done in Foundational Infrastructure Working Group Oct 30, 2025
@rkoster rkoster deleted the update-windows-syscalls branch October 30, 2025 16:28
@jorbaum
Copy link
Contributor

jorbaum commented Oct 31, 2025

@aramprice I guess since you got this merged we can revert my change and increase windows-2022 to windows-latest again?

See also #77 (comment)

@aramprice
Copy link
Member Author

@jorbaum - good call out, thank you. Created #82

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants