Attention is currently required from: fixeria, pespin.

daniel has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/32536 )

Change subject: osmo_io: Add io_uring backend
......................................................................


Patch Set 11:

(15 comments)

File src/core/osmo_io_uring.c:

https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/2cb864d2_dc8a8633
PS10, Line 118: return
> Looks like `msg` is leaked here. […]
I also combined submit_read() and submit_recvmsg() into one function since they 
are nearly identical


https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/5a0f9c27_c3519917
PS10, Line 126: Could not get io_uring_sqe
> missing `\n`
Done


https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/0b8eac9f_12888db1
PS10, Line 130:         // Prep msgb/iov
> cosmetic: use the `/*  ... […]
Done


https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/4c811e23_9aec738b
PS10, Line 135:         // NOTE: This only works if we have one read per fd
> cosmetic: use the `/*  ... […]
Done


https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/c4490aee_cb46df3b
PS10, Line 153: return
> Again, looks like `msg` is leaked here.
Done


https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/84577c9c_84851497
PS10, Line 165: Could not get io_uring_sqe
> Again missing `\n`.
Done


https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/1ac096b2_2394de44
PS10, Line 275: /* Fallthrough */
> This comment is not really needed since there is no code preceding it.
Done


https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/cd73e497_51a04bba
PS10, Line 295: io_uring_peek_cqe
> Are you sure this is correct? […]
No, it looks wrong. Seems like I still got all cqes because peek would 
(eventually) return the same completion until its io_uring_cqe_seen()


https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/71151ea0_c55ac0f9
PS10, Line 326: Could not get io_uring_sqe
> missing '\n'
Done


https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/fd1a42a5_7defc65a
PS10, Line 360: if (!sqe)
              :                         OSMO_ASSERT(0);
> `OSMO_ASSERT(sqe != NULL)`
Ack


https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/ef12b6cb_28afeab0
PS10, Line 362: (void *)0x0
> Why not `NULL`?
Done


https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/f4f7cacc_5236b267
PS10, Line 369: if (!sqe)
              :                         OSMO_ASSERT(0);
> `OSMO_ASSERT(sqe != NULL)`
Done


https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/517eb248_f5f18ca1
PS10, Line 397: return
> `msg` is leaked here.
Done


https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/34689a92_515ad1e0
PS10, Line 404: Could not get io_uring_sqe
> missing `\n`
Done


https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/2a74067c_3fa9d3b9
PS10, Line 438:
> ws
Done



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/32536
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I5152129eb84b31ccc9e02bc2a5c5bdb046d331bc
Gerrit-Change-Number: 32536
Gerrit-PatchSet: 11
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-CC: daniel <[email protected]>
Gerrit-CC: osmith <[email protected]>
Gerrit-Attention: fixeria <[email protected]>
Gerrit-Attention: pespin <[email protected]>
Gerrit-Comment-Date: Tue, 22 Aug 2023 14:35:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <[email protected]>
Gerrit-MessageType: comment

Reply via email to