Add support for WASI sockets to C API#5624
Conversation
Subscribe to Label Actioncc @peterhuene DetailsThis issue or pull request has been labeled: "wasmtime:c-api"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
Add support for WASI sockets in the C API by adding a new API to handle preopening sockets for clients. This uses HashMap instead of Vec for preopened sockets to identify if caller has called in more than once with the same FD number. If so, then we return false so caller is given hint that they are attempting to overwrite an already existing socket FD.
|
@peterhuene Do you think you would be able to help with this review? |
peterhuene
left a comment
There was a problem hiding this comment.
Hi @font! Thanks for the PR. The changes look good, just some minor feedback comments to go over.
Co-authored-by: Peter Huene <peter@huene.dev>
font
left a comment
There was a problem hiding this comment.
@peterhuene thank you for your review! I've incorporated all of your feedback in new commits that should be squashed and this is now ready for another review.
|
@peterhuene Do you think you would be able to help re-review? |
|
Shoot, sorry @font, this fell off my radar. I'll review it shortly. |
peterhuene
left a comment
There was a problem hiding this comment.
👍 Thanks for implementing this!
This adds support for WASI sockets to the C API by adding a new API to handle preopening sockets for clients. This uses
HashMapinstead ofVecfor preopened sockets to identify if the caller has called in more than once with the same FD number. If so, then we return false so caller is at least given a hint that they may be misusing the API e.g. attempting to overwrite an already existing socket FD.I stuck with the same
boolreturn value because it was already being done for the other APIs e.g.wasi_config_preopen_dir. I think it would be beneficial to have a more specific error type, if possible, but not something to address in this PR.I don't think there are any tests for these APIs so none were added. Of course I was personally testing this with the
cruncontainer runtime that is using these APIs.I don't know who could review this so any help identifying the right people would be greatly appreciated!
Fixes #5071