-
Notifications
You must be signed in to change notification settings - Fork 749
Added http downloader and multicast socket options #1467
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
Added http downloader and multicast socket options #1467
Conversation
11c8b4c to
bd711a8
Compare
bd711a8 to
c13ff47
Compare
|
|
||
| static inline __wasi_errno_t | ||
| __wasi_sock_get_reuse_port(__wasi_fd_t fd, int8_t *reuse) | ||
| __wasi_sock_get_reuse_port(__wasi_fd_t fd, uint8_t *reuse) |
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 there a particular reason about from int8_t to uint8_t
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.
Only reason is that for other functions I had used uint8_t for the enable/disable flag and I thought I would change this for consistency with those other set/get sock opts
| __import_name__("sock_set_keep_alive"))); | ||
|
|
||
| static inline __wasi_errno_t | ||
| __wasi_sock_set_keep_alive(__wasi_fd_t fd, uint8_t option) |
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.
may I ask about possible values of option?
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.
Just 0 and 1 like a bool. Wondering now whether I can just use std bool for the argument here. As it's be clearer what the values can be without a comment. Will have a look
|
@loganek @cimacmillan We have created the release WAMR-1.0.0, and merged the changes from branch main into branch dev/socket, as there is a conflict in file |
|
Sure, thanks for letting us know |
bfa6d70 to
0e84ec3
Compare
loganek
left a comment
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.
Thanks for the change, overall looks good to me. I left a couple of comments. The major question I have is whether we could simplify some of the get/set functions that are different in BSD sockets and have single function with address family as an argument instead of two separate functions for every getter setter?
0e84ec3 to
5b80c8f
Compare
5b80c8f to
6972aed
Compare
|
LGTM |
loganek
left a comment
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.
Thanks, looks good. @wenyongh can we merge that?
Implement more socket APIs, refer to #1336 and below PRs: - Implement wasi_addr_resolve function (#1319) - Fix socket-api byte order issue when host/network order are the same (#1327) - Enhance sock_addr_local syscall (#1320) - Implement sock_addr_remote syscall (#1360) - Add support for IPv6 in WAMR (#1411) - Implement ns lookup allowlist (#1420) - Implement sock_send_to and sock_recv_from system calls (#1457) - Added http downloader and multicast socket options (#1467) - Fix `bind()` calls to receive the correct size of `sockaddr` structure (#1490) - Assert on correct parameters (#1505) - Copy only received bytes from socket recv buffer into the app buffer (#1497) Co-authored-by: Marcin Kolny <mkolny@amazon.com> Co-authored-by: Marcin Kolny <marcin.kolny@gmail.com> Co-authored-by: Callum Macmillan <callumimacmillan@gmail.com>
…1467) Add a group of socket options used by cURL and rust stdlib, as well as some UDP multicast options.
Implement more socket APIs, refer to bytecodealliance#1336 and below PRs: - Implement wasi_addr_resolve function (bytecodealliance#1319) - Fix socket-api byte order issue when host/network order are the same (bytecodealliance#1327) - Enhance sock_addr_local syscall (bytecodealliance#1320) - Implement sock_addr_remote syscall (bytecodealliance#1360) - Add support for IPv6 in WAMR (bytecodealliance#1411) - Implement ns lookup allowlist (bytecodealliance#1420) - Implement sock_send_to and sock_recv_from system calls (bytecodealliance#1457) - Added http downloader and multicast socket options (bytecodealliance#1467) - Fix `bind()` calls to receive the correct size of `sockaddr` structure (bytecodealliance#1490) - Assert on correct parameters (bytecodealliance#1505) - Copy only received bytes from socket recv buffer into the app buffer (bytecodealliance#1497) Co-authored-by: Marcin Kolny <mkolny@amazon.com> Co-authored-by: Marcin Kolny <marcin.kolny@gmail.com> Co-authored-by: Callum Macmillan <callumimacmillan@gmail.com>
Added a group of socket options that I found cURL & rust stdlib uses, as well as some UDP multicast options.
#1336