fix(network): port bsdsocket from latest Amiberry - #39
Conversation
Old host layer crashed Amiga apps on connect (#28, AmiTradeCenter). Port bsdsocket.cpp/.h and the POSIX half of bsdsocket_host.cpp from current Amiberry; only local adaptation is SDL2 mutex types. Key fixes carried over: - select()/FD_SET guarded against fd >= FD_SETSIZE - ftable writes bounds-checked; stale SET_* flags cleared on close - event monitor unregistered on cleanup/reset (no dangling socketbase) - errno preserved across logging in blocking send/recv/connect paths - setsockopt/getsockopt arg validation, SO_EVENTMASK query support - sendmsg reads tolen from guest memory, not guest pointer - WaitSelect nfds clamped to descriptor table size - dup2socket returns new fd; EBADF on invalid descriptor - gethostname/Inet_LnaOf/Inet_NetOf/Inet_MakeAddr implemented - per-call logging gated behind log_bsd (BSDLOG/BSDTRACE) Refs #28
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f254b521a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- bsdthr_blockingstuff: keep EINTR when a blocking call was aborted via sockabort; the unconditional saved_errno restore reported EAGAIN/EINPROGRESS so callers retried aborted operations - event monitor: treat unconnected datagram sockets as ready for read/write readiness polling (getpeername fails with ENOTCONN on bound UDP sockets, which blocked all event delivery) - event monitor: recv() returning 0 is EOF only on stream sockets; on UDP it is a valid zero-length datagram and must not fire REP_CLOSE
|
@codex review |
Marking unconnected datagram sockets connected=true let the monitor's REP_CONNECT fallback post a false connect-completed event when a broad mask (e.g. REP_ALL) included REP_CONNECT. dgram now relaxes only the read/write readiness gates; connected keeps the getpeername() result.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe7e476233
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Summary
Ports the bsdsocket.library emulation from the latest Amiberry to fix the crash-on-connect reported in #28 (AmiTradeCenter 1.4, OS3.9/Pimiga 5).
Files changed:
src/bsdsocket.cpp— identical to current Amiberrysrc/include/bsdsocket.h— identical to current Amiberrysrc/osdep/bsdsocket_host.cpp— upstream POSIX section; only local adaptation is SDL2 mutex types (SDL_mutex/<SDL_mutex.h>instead of SDL3)No SLIRP/PCAP code involved (both remain compiled out).
Key fixes carried over
select()/FD_SETguarded againstfd >= FD_SETSIZE(host-side crash)post_socket_event()bounds-checksftablewrites; staleSET_*flags cleared on close/unregistersocketbase*afterhost_sbreset()(was an empty stub here)setsockopt/getsockoptargument-size validation;SO_EVENTMASKquery supportsendmsgreadstolenfrom guest memory instead of passing a guest pointerhost_sendtochecks the looked-up socket, not a stalesb->sWaitSelectclampsnfdsto the descriptor table sizedup2socketreturns the new descriptor;EBADFon invalid fdgethostname,Inet_LnaOf,Inet_NetOf,Inet_MakeAddrimplemented correctlylog_bsd(BSDLOG/BSDTRACE) to stop log floodingVerification
socket_emu=true: ~90s run, no errors, clean exit on SIGTERMbsdsocket.cpp/.hbyte-identical; host file differs by exactly the 2 SDL2 linesThe literal AmiTradeCenter-on-OS3.9 repro needs the reporter's environment — @RWXecute a retest with a build from this branch would confirm.
Fixes #28