Add merge commit for dev/socket #1541
Conversation
Implement wasi_addr_resolve function.
Also slightly modify the interface to make it more accessible for the end user:
- replace port argument with the service - so the user can actually get the port for a given service if unknown
- introduce __wasi_addr_info_t and use it as a buffer for addresses, instead of generic buffer
- introduce __wasi_addr_info_hints_t so user can enable filtering on the syscall level (and therefore use smaller buffers for addresses)
- add max_size parameter for the API as an output - in case the number of addresses is bigger than the buffer size, user can repeat the call with bigger buffer
This change is very minimalistic, and it doesn't include the followings:
1. implementation of getaddrinfo in the lib-socket
2. sample application
Which are to be added in the following change bytecodealliance#1336
…ytecodealliance#1327) Fix socket-api byte order issue for systems where host byte order and network byte order are the same: - Document data structures used for storing IP addresses - Fix bug in bind and connect methods by updating code in wasi_socket_ext
Slightly change the __wasi_sock_addr_local interface - since we already have a `__wasi_addr_t` structure which is an union, there's no need for passing the length around - the address buffer will always have the right length (i.e. max of all address families).
Slightly changed the interface sock_addr_remote - since we already have a `__wasi_addr_t` structure which is an union, there's no need for passing length around - the address buffer will always have the right length (i.e. max of all address families).
Merge main into dev/socket
For now this implementation only covers posix platforms, as defined in MVP bytecodealliance#1336
The ns-lookup accepts domain names as well as suffixes, e.g.: ``` --allow-resolve=* # allow all domain names --allow-resolve=example.com # only allow example.com name resolution --allow-resolve=example.com --allow-resolve=*.example.com # allow example.com and its subdomains' name resolution ```
So getaddrinfo() can be used when compiling wasm app of C programs.
Added socket send and recv timeout options with implementation for posix platform. This is part of a extending support for sockets in WASI. bytecodealliance#1336. Also add sample that sets and reads back the send and receive timeouts using the native function binding.
…1467) Add a group of socket options used by cURL and rust stdlib, as well as some UDP multicast options.
bytecodealliance#1490) For some implementations (e.g. Mac OS) `bind()` requires the length to be exactly equal to either `sockaddr_in` or `sockaddr_in6` structure. Because we always used `sizeof(struct sockaddr_storage)`, `bind()` was returning errors. In this change we fix the behavior. See StackOverflow [1] for details. [1] https://stackoverflow.com/questions/73707162/socket-bind-failed-with-invalid-argument-error-for-program-running-on-macos
This also fixes debug build
…ytecodealliance#1497) **What** * Updated `copy_buffer_to_iovec_app` so that it copies as much of the buffer into the iovec as specified * Throw invalid value when allocating an iovec of size 0 **Why** * A bug found from TCP client example which allocates 1024 for the iovec size (where the buf size is also 1024) but received bytes is passed in as the `buf_size` argument to `copy_buffer_to_iovec_app`. This would return early after hitting this check `buf + data->buf_len > buf_begin + buf_size`. However, if the amount to copy is less than the iovec size, we should copy that much of the buf size. Eg TCP client sample receives 27(?) bytes at a time, and this copies 27 bytes into the iovec of size 1024 * The TCP client example attempts to recv bytes of size 0, this attempts to wasm malloc size 0, which outputs a warning. We should early return if recv bytes of size 0
|
@loganek Since development branch might have many temporarily/unused commits, we usually |
|
@wenyongh thanks a lot. I agree that development branches might be a bit polluted and contain a lot of 'ugly' commits. I think the way we could possibly approach it is to give developers who work on those branches write access (with force push option) to those particular branches so once the feature is completed, they can polish the history. What are your thoughts on that? |
|
maybe use |
|
Yes, but ideally this should be done by the developers working on the PR, not necessarily by the maintainers (as developers probably know best what changes should go as part of one commit, and what should not). It will also reduce the maintainers being a bottleneck, as sometimes rewriting a history takes quite a bit of effort. That's why I suggested giving write access to dev branches to developers so once the change is completed they can:
so then maintainers can merge with |
Unfortunately, we have no privilege to change the setting. I think we can ask the developer to submit relatively high quality commits, or let him roll up until a necessary change is reached. And we will polish the commit title and comments before merging it. |
|
ah, I see. I thought github allows to grant per-branch permissions. I guess one other option is for developers to then pull changes into their repository, do the cleanup and make a final PR from their fork to the WAMR's |
|
Close this PR as similar PR #1542 had been merged. |
4 participants
This change only introduces a merge commit for
dev/socketbranch and does not include any file changes. I noticed that thedev/socketwork was squashed which means we're loosing a history of the work being done (in case we removedev/socketbranch). I think for the future workstreams we should aim to avoid squashing commits when merging larger branch and instead one of the below (in that order):So we always have full history. Having said that, we should ask contributors to clean-up their individual PRs before they're merged (e.g. ask them to squash some of the commits) but I don't think maintainers should do that by themselves. Worth noting that squashing commits also overrides authors of the change in git log, and even if contributors are listed in the log message, it's not obvious to find their changes.