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

zephyr: add WASI socket support #3335

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

lucasAbadFr
Copy link

To address #3311

@@ -3216,7 +3216,7 @@ wasm_runtime_init_wasi(WASMModuleInstanceCommon *module_inst,
goto fail;
}
addr_pool_inited = true;

#if !defined(BH_PLATFORM_ZEPHYR)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you implement os_convert_stdin_handle, os_convert_stdout_handle, os_convert_stderr_handle and some other APIs if needed? It is strange that these critical codes are commented for zephy.

@@ -196,8 +196,13 @@ static inline bool
cond_timedwait(struct cond *cond, struct mutex *lock, uint64_t timeout,
bool abstime) REQUIRES_EXCLUSIVE(*lock) NO_LOCK_ANALYSIS
{
#if defined(BH_PLATFORM_ZEPHYR)
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Will you implement it in the future? How about:

#if defined(BH_PLATFORM_ZEPHYR) 
    return false;
#endif

@@ -22,6 +22,7 @@
#include "refcount.h"
#include "rights.h"
#include "str.h"
#include "assert.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems no assert function is called? And no need to include "assert.h", just use bh_assert if needed.

@@ -647,7 +648,7 @@ fd_table_insert(wasm_exec_env_t exec_env, struct fd_table *ft,
fd_object_release(exec_env, fo);
return convert_errno(errno);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Had better not add trailing spaces, or code guideline check CI will report error.

@@ -778,6 +779,7 @@ fd_object_get_locked(struct fd_object **fo, struct fd_table *ft, __wasi_fd_t fd,
__wasi_rights_t rights_inheriting)
TRYLOCKS_EXCLUSIVE(0, (*fo)->refcount) REQUIRES_EXCLUSIVE(ft->lock)
{
printf("[SSP] fd_object_get_locked\n");
Copy link
Contributor

Choose a reason for hiding this comment

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

These printf will be removed in the future, right?
And had better use os_printf or LOG_XXX instead if needed, printf may be not supported in some platforms.

src_addr->addr.ip4.addr.n1 = adjusted_addr_copy[2];
src_addr->addr.ip4.addr.n2 = adjusted_addr_copy[1];
src_addr->addr.ip4.addr.n3 = adjusted_addr_copy[0];
src_addr->addr.ip4.port = adjusted_port_copy;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

@@ -0,0 +1,17 @@
#include "platform_api_extension.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Had better add license header

@@ -0,0 +1,300 @@
#include "platform_api_extension.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Had better add license header

@@ -0,0 +1,1068 @@
#include "platform_api_extension.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Had better add license header

adjusted_addr.addr.ip4.addr.n1 = ip_addr[2];
adjusted_addr.addr.ip4.addr.n2 = ip_addr[1];
adjusted_addr.addr.ip4.addr.n3 = ip_addr[0];
adjusted_addr.addr.ip4.port = ntohs(*port);
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

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.

None yet

2 participants