Add ProcessInfo implementations for other platforms #559
Conversation
|
@swift-ci please test |
|
@swift-ci please test Windows |
| return "" | ||
| #elseif os(Windows) | ||
| return withUnsafeTemporaryAllocation(of: wchar_t.self, capacity: 1040) { usernameBuffer in |
There was a problem hiding this comment.
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.
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
Co-authored-by: Saleem Abdulrasool <[email protected]>
| @@ -117,7 +117,11 @@ final class _ProcessInfo: Sendable { | |||
| } | |||
|
|
|||
| var processIdentifier: Int32 { | |||
| return getpid() | |||
| #if os(Windows) | |||
| return Int32(GetProcessId(GetCurrentProcess())) | |||
There was a problem hiding this comment.
This is unsafe isn't it? If the high bit is set, this will trap?
There was a problem hiding this comment.
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.
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.
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.
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.
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
|
@swift-ci please test |
|
BTW, I do have some changes related to this area that I've put up as a draft at #561 I'll see if I can pull them over here as your change is more comprehensive. |
Co-authored-by: Saleem Abdulrasool <[email protected]>
Sounds good! I accepted your two changes above and the other changes make sense to pull in - let me know if you want me to pull them in myself or if you're planning to post suggestions to do it |
If you could pull them in, that would be wonderful! |
Done! I think all of the changes from that draft PR should now be combined into this one |
|
@swift-ci please test |
|
@swift-ci please test Windows |
|
@swift-ci please test |
|
@swift-ci please test Windows |
|
@swift-ci please test Windows |
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
|
@swift-ci please test |
This adds the implementations of
ProcessInfoAPIs for other platforms such as Windows and WASI (copied from swift-corelibs-foundation) so that this type can fully replace theProcessInfopresent in swift-corelibs-foundation