Skip to content
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

Fix Socket#tty? to false on Windows #13175

Merged
merged 4 commits into from
Mar 13, 2023

Conversation

Blacksmoke16
Copy link
Member

@Blacksmoke16 Blacksmoke16 commented Mar 10, 2023

This ended up being not as straight forward as the FileDescriptor context as fd in this case is actually a LibC::SOCKET not a raw Int like in the other context. After doing some research it seems that sockets can be treated like other windows handles and as such can pass it to _open_osfhandle in order to get a C file descriptor to pass to _isatty.

After this PR, the spec I added correctly returns false, tho I'm not sure if there is a case where you can get a socket to be a tty in order to test the case of it being true.

Fixes #13165

refs:

EDIT: After reading some more I'm not so sure:

The _open_osfhandle call transfers ownership of the Win32 file handle to the file descriptor. To close a file opened by using _open_osfhandle, call _close. The underlying OS file handle is also closed by a call to _close.

Transferring the ownership of the handle to the FD doesn't seem like what we want, and can't really close it w/o also closing the underlying handle?

EDIT2: Just going to try returning false given it doesn't seem like you can have a windows socket be a tty. Can revisit in future if this is disproven, but at least gets things compiling.

@beta-ziliani beta-ziliani added this to the 1.8.0 milestone Mar 10, 2023
@straight-shoota straight-shoota changed the title Correctly use _isatty for sockets on windows Fix Socket#tty? to false on Windows Mar 10, 2023
@straight-shoota straight-shoota merged commit b992e0b into crystal-lang:master Mar 13, 2023
@Blacksmoke16 Blacksmoke16 deleted the windows-isatty branch April 2, 2024 16:50
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: undefined fun 'isatty' for LibC on Windows
4 participants