Attention is currently required from: pespin, daniel. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/32536 ) Change subject: osmo_io: Add io_uring backend ...................................................................... Patch Set 10: Code-Review-1 (15 comments) File src/core/osmo_io_uring.c: https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/7b908048_52ef20e0 PS10, Line 118: return Looks like `msg` is leaked here. Why not just doing `OSMO_ASSERT(msghdr != NULL)` here and above? https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/08e1b5a8_82b81a6d PS10, Line 126: Could not get io_uring_sqe missing `\n` https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/14e794d5_0ee1be02 PS10, Line 130: // Prep msgb/iov cosmetic: use the `/* ... */` syntax for comments https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/fdf60304_f81b136b PS10, Line 135: // NOTE: This only works if we have one read per fd cosmetic: use the `/* ... */` syntax for comments https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/d41c243f_84af22ea PS10, Line 153: return Again, looks like `msg` is leaked here. https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/5325b092_4262a866 PS10, Line 165: Could not get io_uring_sqe Again missing `\n`. https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/6b190445_c091700c PS10, Line 275: /* Fallthrough */ This comment is not really needed since there is no code preceding it. https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/d08ec083_f1575d91 PS10, Line 295: io_uring_peek_cqe Are you sure this is correct? This function is called in the conditional part of the loop, and then here again? https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/55656c2e_28db9f0a PS10, Line 326: Could not get io_uring_sqe missing '\n' https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/0052e548_5fbcfce6 PS10, Line 360: if (!sqe) : OSMO_ASSERT(0); `OSMO_ASSERT(sqe != NULL)` https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/4a83f92c_c58b12e1 PS10, Line 362: (void *)0x0 Why not `NULL`? https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/7671a770_7cc3fa79 PS10, Line 369: if (!sqe) : OSMO_ASSERT(0); `OSMO_ASSERT(sqe != NULL)` https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/edcfbf5a_a7da4d60 PS10, Line 397: return `msg` is leaked here. https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/c8418e78_b814190e PS10, Line 404: Could not get io_uring_sqe missing `\n` https://gerrit.osmocom.org/c/libosmocore/+/32536/comment/ba3a389e_cdf33736 PS10, Line 438: ws -- 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: 10 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Sat, 12 Aug 2023 06:36:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment