Preface: What happened here was I was wondering if we could get rid of
the reference out parameter, and the LLM went in a different direction
than I imagined.
I thought the benefit would just be skipping some of the checks
about the local-side socket that unp_connectat starts with, since
uipc_sosend_dgram would always be leaving us in a more-known state.
The LLM agreed with that, though it is not clear to me that the elision
of some of those checks is totally legit. It also went further trying to
make uipc_sosend_dgram more stateless and also skip the
unp_connect_peer final third of unp_connectat. That was unexpected
to me!
I consider it poor form to submit patches I don't fully understand, so
I am sorry to be violating my own precept. And certainly everyone should
feel free to ignore this one accordingly. Nonetheless I am submitting it
because if it works as advertised --- both getting rid of the hacky
extra param, and perhaps improving performance/concurrency --- that's a
pretty neat double win.
uipc_sosend_dgram() reached a peer named by sendto(2) by calling
unp_connectat(), and undid it with unp_disconnect() a few lines later.
Nothing of the connection was used in between: the enqueue selects
&so2->so_rcv, the peer's shared receive buffer, precisely because the
send is unconnected, and the per-sender buffer linkage a connection
establishes is guarded by addr == NULL.
Resolve the peer directly instead. unp_resolve_peer() already returns a
referenced peer socket, so the path becomes: resolve, check the socket
type, lock the pair, enqueue, unlock and release. That spares every
unconnected datagram an unp_connect2() and unp_disconnect() pair.
It also removes the mechanism that existed only to serve this caller.
unp_connectat() no longer takes an out pointer to hand back a referenced
peer with its PCB left locked, and unp_connect_peer() no longer takes
return_peer_locked, this having been its only user.
Two errors the temporary connection used to raise are now raised
explicitly: an empty sun_path gives EINVAL, since AT_FDCWD cannot
name a peer by descriptor, and an already connected socket gives
EISCONN. dgram_sendto_empty_path and dgram_sendto_connected cover
them. dgram_sendto_self covers a socket naming itself, the one case
where the pair of PCBs to lock is a single PCB. Two others go away: a
concurrent connect(2) no longer fails the send with EALREADY, and the
send no longer sets UNP_CONNECTING, so it no longer serialises against
one.
Concurrent sends are unaffected, SOCK_IO_SEND_LOCK() being held across
the whole send: a second sendto(2) blocks there regardless. The sending
socket's unp_conn is no longer transiently set either, so
getpeername(2) on it can no longer observe a peer that is not there.
Signed-off-by: John Ericson <John.Ericson@Obsidian.Systems>
Assisted-by: Claude Code (Claude Opus 4.8 and Fable 5)