Update zephyr file socket branch#4406
Update zephyr file socket branch#4406lum1n0us merged 9 commits intobytecodealliance:dev/zephyr_file_socketfrom
Conversation
Fixed issues in os_renameat Signed-off-by: Stephen Berard <stephen.berard@outlook.com>
Addressed numerous of build warnings on Zephyr Signed-off-by: Stephen Berard <stephen.berard@outlook.com>
Signed-off-by: Stephen Berard <stephen.berard@outlook.com>
Signed-off-by: Stephen Berard <stephen.berard@outlook.com>
Partial implementation — just avoids a hard fault.
lum1n0us
requested review from
TianlongLiang,
loganek,
no1wudi,
wenyongh,
xujuntwt95329 and
yamt
as code owners
| exec_env = wasm_runtime_get_exec_env_singleton(module_inst); | ||
| if (exec_env) { | ||
| wasm_runtime_dump_mem_consumption(exec_env); | ||
| (WASMModuleInstance *)module_inst->cur_exception |
There was a problem hiding this comment.
Hi @lum1n0us, I don't understand what you mean by the above. I don't believe this file was updated as part of this Zephyr work. Can you explain more what you would like to see changed here?
There was a problem hiding this comment.
I used to think it was a typo and that we should just remove it.
| // Proper file descriptor on which we can poll(). | ||
| pfds[i] = (os_poll_file_handle){ | ||
| .fd = fos[i]->file_handle, | ||
| .fd = fos[i]->file_handle->fd, |
There was a problem hiding this comment.
file_handle's type is os_file_handle which is typedef int os_file_handle. So a compilation error will be raised: error: invalid type argument of ‘->’ (have ‘os_file_handle’ {aka ‘int’}).
There was a problem hiding this comment.
In #3633, @lucasAbadFr changed for Zephyr to be the following:
typedef struct zephyr_handle *os_file_handle;
I'll see what I can do to unify this better to align with the POSIX/Linux implementation.
| if (dup == NULL) { | ||
| return NULL; // Allocation failed | ||
| } | ||
|
|
There was a problem hiding this comment.
May need memset to zero out the buffer.
There was a problem hiding this comment.
It shouldn't be needed here. We allocate the string length plus 1 for the terminator. If the allocation fails, the malloc failed and the buffer was not allocated; NULL is returned to indicate error. If the malloc succeeded, memory is allocated and a memcpy is performed for the full length of the buffer (which is strlen + 1). This will copy the full string plus the NULL terminator. A memset prior would not cause an issue but would work done unnecessarily (perf impact).
In the code above, I'm using malloc when I should probably use os_malloc instead. Existing code uses the BH_MALLOC macro (which calls os_malloc). I've update the code to use this instead.
There was a problem hiding this comment.
I just tested this change to BH_MALLOC and it results in a hard-fault on my device. I have not tracked down the root cause, however it appears to be when the duplicated string is accessed.
There was a problem hiding this comment.
Apologies, the above can be ignored. My issue turned out to be something else memory related in my setup.
| GET_FILE_SYSTEM_DESCRIPTOR(old_handle->fd, ptr); | ||
|
|
||
| char *path = strdup(new_path); | ||
| char *path = duplicate_string(new_path); |
There was a problem hiding this comment.
need to free path somewhere
| // va_end(args); | ||
|
|
||
| return ret; | ||
| // return ret; |
There was a problem hiding this comment.
need to remove commented code
lum1n0us
merged commit 442ea20
into
bytecodealliance:dev/zephyr_file_socket
* Added os_is_* methods for stdin/stdout/stderr. Fixed issues in os_renameat * Zephyr platform fixes for WASI sockets Addressed numerous of build warnings on Zephyr * Updated `os_writev` to use `fwrite` for STDOUT/STDERR * Temporarily reverted change to `writev` to work around an issue. * Fixes: fstat, fstatat, and unlink * Add initial support for directories in os_openat. Partial implementation — just avoids a hard fault. Signed-off-by: Stephen Berard <stephen.berard@outlook.com> Co-authored-by: Dan Kouba <dan@atym.io>
* Added os_is_* methods for stdin/stdout/stderr. Fixed issues in os_renameat * Zephyr platform fixes for WASI sockets Addressed numerous of build warnings on Zephyr * Updated `os_writev` to use `fwrite` for STDOUT/STDERR * Temporarily reverted change to `writev` to work around an issue. * Fixes: fstat, fstatat, and unlink * Add initial support for directories in os_openat. Partial implementation — just avoids a hard fault. Signed-off-by: Stephen Berard <stephen.berard@outlook.com> Co-authored-by: Dan Kouba <dan@atym.io>
3 participants
This is based on #4377.
@srberard Please help me confirm this update. This PR is supposed to contain all and only your latest modifications. If it is as expected, I will merge this PR while ignoring the CI results. I hope we can use new PRs to fix those CI failures.