Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 ProcessInfo implementations for other platforms #559
Add ProcessInfo implementations for other platforms #559
Changes from 6 commits
2935426
e53829e
6f6d98b
cd76481
e7670ad
942eb1e
4bc81d5
46ceb2d
855f2c9
0484ea9
f0bd1fe
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This is unsafe isn't it? If the high bit is set, this will trap?
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.
Yes it will trap if high bits are set. Is it possible for Windows to return a process ID with high bits set? (on Darwin
pid_t
is defined as a 32-bit signed integer). If so, is there a behavior here that you think would be better? (I believe in SCF it will trap today if the high bits are set)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.
Yeah, it is. The ProcessId is a DWORD (UInt32). I guess we could do a
Int32(bitPattern:)
initializer?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.
We could, but that would be a behavioral change from SCF today. Do you think that change / this new behavior would be expected for Windows clients of Foundation?
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.
I think that it would be more expected than a trap which would not make sense - I didn't create an
Int32
, why is this trapping?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.
Ok I can update that to use a bit pattern then if you feel that'd be the better, more expected behavior for our windows clients
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.
Is this meant to be the user that the process is running as or whatever user is logged in? I wonder if we should query the information from the process.
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.
I believe it should be the "current user" (i.e. whichever user is logged in) based on what I've heard from @iCharlesHu - this code was mostly just copied from swift-corelibs-foundation as-is so I think it should behave the same as what SCF currently does