Add OpenStandardInputHandle, OpenStandardOutputHandle, and OpenStandardErrorHandle APIs#123478
Add OpenStandardInputHandle, OpenStandardOutputHandle, and OpenStandardErrorHandle APIs#123478
Conversation
…rdErrorHandle APIs Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
adamsitnik
left a comment
There was a problem hiding this comment.
@copilot please address my feedback
…ify tests Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
adamsitnik
left a comment
There was a problem hiding this comment.
@copilot please address my feedback
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
adamsitnik
left a comment
There was a problem hiding this comment.
@copilot please address my feedback
src/libraries/Common/src/Interop/Unix/System.Native/Interop.Fcntl.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
adamsitnik
left a comment
There was a problem hiding this comment.
I wrapped my head around error handling. Long story short, we need to keep throwing the same exception in all the places that were using Interop.Sys.FileDescriptors so far.
Before the introduction of the new public API, we were using Interop.CheckIo(Interop.Sys.Dup(FD)).
Dup would fail with EBADF if FD was invalid, so is fcntl. We need to return -1 in SystemNative_FcntlCheckAccess when fcntl fails.
When the access rights are not as requested, the SystemNative_FcntlCheckAccess should also set errno to EBADF and return -1. To mimic read/write sys-calls.
So SystemNative_FcntlCheckAccess should not return bool, but failure (invalid fd, wrong access) or success.
The OpenStandardInputHandle, OpenStandardOutputHandle and OpenStandardErrorHandle form ConsolePal.Unix.cs should accept an optional boolean parameter bool verifyFd: true. And When the call to SystemNative_FcntlCheckAcces fails, they should just call Interop.CheckIo when verifyFd is true.
When verifyFd is false, the method should just return raw handle value. Something like this:
public static SafeFileHandle OpenStandardInputHandle(bool verifyFd = true) => OpenStandardHandle(0, FileAccess.Read, verifyFd);
public static SafeFileHandle OpenStandardOutputHandle(bool verifyFd = true) => OpenStandardHandle(1, FileAccess.Write, verifyFd);
public static SafeFileHandle OpenStandardErrorHandle(bool verifyFd = true) => OpenStandardHandle(2, FileAccess.Write, verifyFd);
private static SafeFileHandle OpenStandardHandle(IntPtr fd, FileAccess access, bool verifyFd)
{
if (verifyFd)
{
Interop.CheckIo(Interop.Sys.Fcntl.CheckAccess(fd, (int)access));
}
return new SafeFileHandle(fd, ownsHandle: false);
}Now, all the places that were using Interop.Sys.FileDescriptors.STDIN_FILENO and similar directly like here, should pass verifyFd: false to ensure they don't suddenly start throwing an exception:
public static bool IsInputRedirectedCore()
{
- return IsHandleRedirected(Interop.Sys.FileDescriptors.STDIN_FILENO);
+ return IsHandleRedirected(OpenStandardInputHandle(verifyFd: false));
}This should allow to not introduce any breaking changes into existing public API
|
@copilot please address my feedback from #123478 (review) (I forgot to tag you) |
Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
…soleStream to BrowserConsoleStream Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
…orms Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jkotas
left a comment
There was a problem hiding this comment.
LGTM!
Please check that the new tests pass in runtime-extra-platforms pipeline before merging.
|
/ba-g none of the failures in extra platforms look related to this change. Mostly infrastructure issues. |
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.