Skip to content

Dev/zephyr file socket#4377

Merged
lum1n0us merged 9 commits intobytecodealliance:dev/zephyr_file_socketfrom
atym-io:dev/zephyr_file_socket
Aug 1, 2025
Merged

Dev/zephyr file socket#4377
lum1n0us merged 9 commits intobytecodealliance:dev/zephyr_file_socketfrom
atym-io:dev/zephyr_file_socket

Conversation

@srberard
Copy link
Contributor

@srberard srberard commented Jun 17, 2025

This is still a work in progress, but committing now to make the code available for others who might want to review, test, or build on it while it's still being developed.

Fixes include the following (Zephyr target):

  • Improved Socket support
  • Fixes to stdout/console output
  • Fixes for fstat, fstatat, and unlink
  • Code changes to fix numerous build warnings.

This branch still needs to be updated to the latest trunk and validation on non-Zephyr platforms.

@lum1n0us lum1n0us mentioned this pull request Jun 17, 2025
Merged
@lum1n0us
Copy link
Contributor

lum1n0us commented Jun 17, 2025

@banana-sun Please conduct a test with this PR, thanks.

@lum1n0us
Copy link
Contributor

lum1n0us commented Jun 17, 2025

@srberard There are so many commits that make reviews difficult. Do you think a rebase (targeting main) for bytecodealliance:dev/zephyr_file_socket would make it easier?

@srberard
Copy link
Contributor Author

srberard commented Jun 17, 2025

@srberard There are so many commits that make reviews difficult. Do you think a rebase (targeting main) for bytecodealliance:dev/zephyr_file_socket would make it easier?

I did rebase this a few weeks back off main. I can do that again, but I don't think that will reduce the commit list.

@lum1n0us lum1n0us mentioned this pull request Jun 18, 2025
Merged
6 tasks
@lum1n0us
Copy link
Contributor

lum1n0us commented Jun 20, 2025 •
edited
Loading

@srberard I've updated dev/zephyr_file_socket by merging the latest commits (up to 64ef369) from main via #4384. Please update this PR, or I can help by updating it directly. ⏩ #4406

This was referenced Jun 22, 2025
Open
Open
@lum1n0us lum1n0us mentioned this pull request Jun 24, 2025
Merged
@lum1n0us
Copy link
Contributor

lum1n0us commented Jun 26, 2025

I've merged #4406. This PR should be rebased.

@lum1n0us
Copy link
Contributor

lum1n0us commented Jun 26, 2025

FYI: #4406 (review)

@lum1n0us
Copy link
Contributor

lum1n0us commented Jun 26, 2025

I guess 64fa157 needs to be merged.

@srberard
Copy link
Contributor Author

srberard commented Jun 26, 2025

@lum1n0us I'm working on this today and will get these conflicts resolved.

@lum1n0us
Copy link
Contributor

lum1n0us commented Jun 26, 2025

Thanks. Please be aware that I've merged #4406, and dev/zephyr_file_socket has been updated. This PR, #4377, needs a rebase.

dkouba-atym and others added 4 commits June 28, 2025 11:18
@dkouba-atym @srberard
Directory support WIP
7556473 
readdir works but I think we have a memory leak somewhere
@dkouba-atym @srberard
Fix: always use standard stream fds. Fix unlinkat and renameat
4ac15c4 
fd 0, 1, and 2 will always be supplied for standard streams. Unlinkat and renameat work exclusively based on supplied paths.
@srberard
Fixed clang formatting
bfb04bf
@srberard
Addressed several review issues from @ lum1n0us
db01b09
@srberard
Copy link
Contributor Author

srberard commented Jun 28, 2025

All the review items identified so far have been address with one exception. This will require some refactoring. I will get this done shortly and update this branch.

@lum1n0us
Copy link
Contributor

lum1n0us commented Jul 9, 2025 •
edited
Loading

All the review items identified so far have been address with one exception. This will require some refactoring. I will get this done shortly and update this branch.

@srberard Please inform me when the PR is ready for review.

@srberard
Copy link
Contributor Author

srberard commented Jul 9, 2025

Hi @lum1n0us,
It will take a bit more time to refactor this completely. As a short-term work-around, I can fix the remaining issue with an ifdef for Zephyr. Then I can do a more comprehensive fix in a separate PR. Let me know if you'd like me to do that.

@lum1n0us
Copy link
Contributor

lum1n0us commented Jul 17, 2025

@srberard I prefer the short-term workaround. Instead of using #ifdef, marking unfinished issues with /*FIXME*/ is acceptable. We're planning to merge this PR into a development branch, and it's permissible to have some CI issues temporarily. Once it's allowed, the community can help contribute patches as well. I also prefer using small PRs to address issues one by one rather than a comprehensive PR all at once.

srberard added 3 commits July 30, 2025 11:09
@srberard
Added a temporary workaround for issue identified in PR#4377
3fc5b95 
Signed-off-by: Stephen Berard <stephen.berard@outlook.com>
@srberard
Fixed syntax error
1e3faec 
Signed-off-by: Stephen Berard <stephen.berard@outlook.com>
@srberard
Fixed reference to file descriptor and cleaned up dead/commented code.
637516b 
Signed-off-by: Stephen Berard <stephen.berard@outlook.com>
@srberard
Copy link
Contributor Author

srberard commented Jul 30, 2025

@lum1n0us I apologize for the delay getting this fix in; I was pulled into another effort. I put in a short term-fix for the outstanding issue. This code now works on POSIX and Zephyr without error. I'll file a separate issue to refactor the Zephyr codebase to align with the POSIX codebase assumptions.

The builds are passing now with one exception that I believe is unrelated to these changes.

@lum1n0us lum1n0us merged commit b8e8230 into bytecodealliance:dev/zephyr_file_socket Aug 1, 2025
546 of 556 checks passed
lum1n0us pushed a commit to lum1n0us/wasm-micro-runtime that referenced this pull request Aug 1, 2025
@srberard @dkouba-atym @lum1n0us
Fix several issues (bytecodealliance#4377)
23637a0 
* Directory support WIP. readdir works but I think we have a memory leak somewhere
* Fix: always use standard stream fds. Fix unlinkat and renameat. fd 0, 1, and 2 will always be supplied for standard streams. Unlinkat and renameat work exclusively based on supplied paths.
* Fix: use macro for free()
* Added a temporary workaround for issue identified in PR#4377
* Fixed reference to file descriptor and cleaned up dead/commented code.

Signed-off-by: Stephen Berard <stephen.berard@outlook.com>
Co-authored-by: Dan Kouba <dan@atym.io>
lum1n0us pushed a commit that referenced this pull request Aug 12, 2025
@srberard @dkouba-atym @lum1n0us
Fix several issues (#4377)
87732f2 
* Directory support WIP. readdir works but I think we have a memory leak somewhere
* Fix: always use standard stream fds. Fix unlinkat and renameat. fd 0, 1, and 2 will always be supplied for standard streams. Unlinkat and renameat work exclusively based on supplied paths.
* Fix: use macro for free()
* Added a temporary workaround for issue identified in PR#4377
* Fixed reference to file descriptor and cleaned up dead/commented code.

Signed-off-by: Stephen Berard <stephen.berard@outlook.com>
Co-authored-by: Dan Kouba <dan@atym.io>
lum1n0us pushed a commit that referenced this pull request Aug 12, 2025
@srberard @dkouba-atym @lum1n0us
Fix several issues (#4377)
6cfc08e 
* Directory support WIP. readdir works but I think we have a memory leak somewhere
* Fix: always use standard stream fds. Fix unlinkat and renameat. fd 0, 1, and 2 will always be supplied for standard streams. Unlinkat and renameat work exclusively based on supplied paths.
* Fix: use macro for free()
* Added a temporary workaround for issue identified in PR#4377
* Fixed reference to file descriptor and cleaned up dead/commented code.

Signed-off-by: Stephen Berard <stephen.berard@outlook.com>
Co-authored-by: Dan Kouba <dan@atym.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@loganek loganek Awaiting requested review from loganek

@lum1n0us lum1n0us Awaiting requested review from lum1n0us

@no1wudi no1wudi Awaiting requested review from no1wudi

@wenyongh wenyongh Awaiting requested review from wenyongh

@xujuntwt95329 xujuntwt95329 Awaiting requested review from xujuntwt95329

@yamt yamt Awaiting requested review from yamt

@TianlongLiang TianlongLiang Awaiting requested review from TianlongLiang

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@srberard @lum1n0us @dkouba-atym