-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Fix sendmsg()
implementation
#24187
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
base: main
Are you sure you want to change the base?
Fix sendmsg()
implementation
#24187
Conversation
7b3c85c
to
7cf5386
Compare
dest
would be called while undefsendmsg()
implementation
@sbc100 I completely rewrote my PR from the ground up. I added a test, changed the fix, and I updated the PR description. |
@sbc100 ci/circleci seems to break because |
src/library_sockfs.js
Outdated
@@ -634,7 +634,8 @@ addToLibrary({ | |||
// connect, and lie, saying the data was sent now. | |||
if (!dest || dest.socket.readyState !== dest.socket.OPEN) { | |||
// if we're not connected, open a new connection | |||
if (sock.type === {{{ cDefs.SOCK_DGRAM }}}) { | |||
if (!dest || sock.type === {{{ cDefs.SOCK_DGRAM }}}) { | |||
sock.type = {{{ cDefs.SOCK_DGRAM }}}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seem lik you could get here with !dest
and with sock.type = {{{ cDefs.SOCK_STREAM }}}
.. in which case this line would change the type of the socket, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but the check was specific for sock.type === {{{ cDefs.SOCK_DGRAM }}}
only, that's why I forced it. Maybe it's ok to keep the socket type, then.
One-char fix!Finally, a little more complicated.
This PR introduces a new test. This test calls
getifaddrs()
, which internally would ultimately callsetmsg()
.The problem was there: calling
getifaddrs()
makesdest
here undefined, butsock.type === {{{ cDefs.SOCK_DGRAM }}}
would betrue
.The current behaviour would skip then the assignement and
dest
would be called just after, but throw asdest
would beundefined
.My solution is to make sure to set
dest
even if thesock.type
matches. Just to be sure, after the condition, I force the sock type. Therefore, I think the intent of #22630 is respected.Fixes #23046.