Attention is currently required from: jolly, laforge.

View Change

4 comments:

• File src/core/osmo_io_uring.c:

  • Patch Set #4, Line 327: if (osmo_fd_is_registered(&iofd->u.uring.connect_ofd))

    Again this is expensive! Should be removed (or conditionally enabled through ifdef).

  • Patch Set #4, Line 341: if (osmo_fd_is_registered(&iofd->u.uring.connect_ofd))

    Again this is expensive! Should be removed (or conditionally enabled through ifdef).

  • Patch Set #4, Line 398: if (osmo_fd_is_registered(&iofd->u.uring.connect_ofd))

    Again this is expensive! Should be removed (or conditionally enabled through ifdef).

  • Patch Set #4, 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 change 35910. To unsubscribe, or for help writing mail filters, visit settings.