Attention is currently required from: jolly, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35910?usp=email )
Change subject: osmo_io: Use poll/select to notify socket connection at osmo_io_uring.c ...................................................................... Patch Set 4: (4 comments) File src/core/osmo_io_uring.c: https://gerrit.osmocom.org/c/libosmocore/+/35910/comment/62900f2c_f007201e PS4, Line 327: if (osmo_fd_is_registered(&iofd->u.uring.connect_ofd)) Again this is expensive! Should be removed (or conditionally enabled through ifdef). https://gerrit.osmocom.org/c/libosmocore/+/35910/comment/0b38f7db_8c1db15b PS4, Line 341: if (osmo_fd_is_registered(&iofd->u.uring.connect_ofd)) Again this is expensive! Should be removed (or conditionally enabled through ifdef). https://gerrit.osmocom.org/c/libosmocore/+/35910/comment/6ebeb0dc_70109e55 PS4, Line 398: if (osmo_fd_is_registered(&iofd->u.uring.connect_ofd)) Again this is expensive! Should be removed (or conditionally enabled through ifdef). https://gerrit.osmocom.org/c/libosmocore/+/35910/comment/cd75808a_4abffff4 PS4, Line 498: /* Use a temporary osmo_fd which we can use to notify us once the connection is established I am not sure this is correct. According to man2 connect, the way to go is: """ The socket is nonblocking and the connection cannot be completed immediately. (UNIX domain sockets failed with EAGAIN instead.) It is pos‐ sible to select(2) or poll(2) for completion by selecting the socket for writing. After select(2) indicates writability, use getsockopt(2) to read the SO_ERROR option at level SOL_SOCKET to determine whether connect() completed successfully (SO_ERROR is zero) or unsuccessfully (SO_ERROR is one of the usual error codes listed here, explaining the reason for the failure). """ I don't see anywhere the mention of "fd becoming readable" as you said. Seems you should be checking the SO_ERROR instead? -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35910?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I4eca9ea72beb0d6ea4d44cce81ed620033f07270 Gerrit-Change-Number: 35910 Gerrit-PatchSet: 4 Gerrit-Owner: jolly <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-CC: pespin <[email protected]> Gerrit-Attention: jolly <[email protected]> Gerrit-Attention: laforge <[email protected]> Gerrit-Comment-Date: Tue, 20 Feb 2024 10:19:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
