serenity
serenity copied to clipboard
Kernel/Net: SO_ERROR reporting for failed O_NONBLOCK socket connections
When using getsockopt
with SO_ERROR
to get the state of a connecting non-blocking socket, the result always stayed EINPROGRESS
no matter if the connection got explicitly refused (RST) or timed out (too many retransmits).
The removed TCPSocket::m_error
code was an abstraction solely used to set SO_ERRORs
for blocking socket connect attempts. The SO_ERROR
assigning logic was moved upstream and now works for both blocking and non-blocking sockets.
Socket::set_so_error()
moved from protected to public which didn't feel good but it does seems like NetworkTask::handle_tcp()
has the right context and is the right place to make that call.
Welcome! :^)
A test is failing, so you might want to make sure everything is in working order first. Additionally, instead of making set_so_error
public, you can mark specific classes as friends to allow them to access private/protected methods.
Didn't think to run the tests locally first, it should be fixed now.
I'm new to C++ and didn't know about friend
yet, thanks for proposing to use it.
Although in this specific situation I couldn't make it work as the function to declare as a friend (handle_tcp
) is static to the NetworkTask.cpp
file. After reading and trying a bunch of different things, I still couldn't come up with a way to make it work. If making Socket::set_so_error()
public isn't acceptable or I just missed how to use friend
for the function, suggestions would be appreciated.
Ah, you wrote NetworkTask::handle_tcp()
so I naively assumed that it was part of NetworkTask
:-) in that case this might be a fine solution for now. I'd like a second opinion on the new error handling though (and losing some more specific errors that we got from handle_tcp
), so I'll leave this open to review.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!
This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions!