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

Feat unix streams #24

Merged
merged 5 commits into from
Sep 13, 2023
Merged

Feat unix streams #24

merged 5 commits into from
Sep 13, 2023

Conversation

Arshia001
Copy link

A few changes related to sockets, unix streams and tty (all needed for crossterm to work):

  • Update IPv6 and unix stream address structs
  • Add address translation between libc <-> wasix for sockaddr_un
  • implement TIOCGWINSZ

addr6.sin6_flowinfo = 0;
addr6.sin6_scope_id = 0;
addr6.sin6_flowinfo = need_revert?htonl(peer_addr->u.inet6.addr.flow_info):peer_addr->u.inet6.addr.flow_info;
addr6.sin6_scope_id = need_revert?htonl(peer_addr->u.inet6.addr.scope_id):peer_addr->u.inet6.addr.scope_id;;
Copy link

Choose a reason for hiding this comment

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

You are sure the need_revert test also apply to those new field?

Copy link
Author

Choose a reason for hiding this comment

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

I'm 99% sure, but let me make 100% sure all the same.

@@ -4,6 +4,8 @@
#define FIONREAD 1
#define FIONBIO 2

#define TIOCGWINSZ 0x101
Copy link

Choose a reason for hiding this comment

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

using random constant can be risky in the future...

Copy link
Author

Choose a reason for hiding this comment

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

Definitely. I did this since the FIONREAD and FIONBIO constants where already redeclared as 1 and 2, which seems random enough; however, I went for an extra 0x100 to keep FIO and TIO kind of distinct. The usual implementation of this constant takes into account things such as the size of the result struct, which I suppose is relevant when implementing the syscall in the kernel. For our usecase, we don't really need the additional data, so I thought a simple number would suffice. I can change it to something else if you feel it's inappropriate.

Copy link

@ptitSeb ptitSeb left a comment

Choose a reason for hiding this comment

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

POSIX expect the port to be in "network order", so bigeeendian. The "need_reverse" test and the reversal of the port is needed there, please keep it.

@Arshia001 Arshia001 force-pushed the feat-unix-streams branch 2 times, most recently from 30b2ff9 to 3c194fb Compare September 13, 2023 16:29
@ptitSeb
Copy link

ptitSeb commented Sep 13, 2023

CI is red, but that seems to be an issue with the CI test iotself?

@Arshia001
Copy link
Author

Yes, the red pipelines are from the original wasi-libc. The one we merged today is green.

@Arshia001 Arshia001 merged commit f2bac4a into main Sep 13, 2023
3 of 6 checks passed
@Arshia001 Arshia001 deleted the feat-unix-streams branch September 13, 2023 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants