Attention is currently required from: daniel, fixeria. pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-abis/+/38927?usp=email )
Change subject: ipaccess: Convert BTS OML & RSL link to use stream_cli ...................................................................... Patch Set 1: (6 comments) File src/input/ipaccess.c: https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/bb9f384d_156b5e3a?usp=email : PS1, Line 494: OSMO_ASSERT(cli); > Why are you so assert()ive? ;) […] If you dive into the code you'd like to put even more asserts! After work on the BSC side I think I indeed can remove this one, will do. https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/bc454569_45f14ee9?usp=email : PS1, Line 545: osmo_stream_cli_send > Why not returning `rc` of this function? because osmo_stream_cli_send() returns void. YES, I KNOW, BIG FAIL!!!!! Remember this case next time you see a new API added returning void 😊 I'm also thinking about creating a osmo_stream_cli_send2() which returns int, and deprectate the other one. That can be done later on though. For osmo_fd backend that was not much of a problem (because we were not checking for a max queue size at the time), but for osmo_io backend it can fail when reaching max length of its internal queue... https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/184dcdbb_cf82fb52?usp=email : PS1, Line 928: *msgb_put(nmsg2, 1) = IPAC_MSGT_ID_ACK; > `msgb_v_put(nmsg2, IPAC_MSGT_ID_ACK)` Done https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/08dd7de2_d0209970?usp=email : PS1, Line 944: uint8_t *data = msgb_l2(msg); > `const`? Done https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/e67b92cd_7ef2b5a0?usp=email : PS1, Line 992: err: : return -1; > `goto`s can be replaced with `return -1`? Yes, but I prefer having all error paths easily visible this way. https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/b122a1da_85c99e60?usp=email : PS1, Line 1014: e1i_ts->line > `s/e1i_ts->line/line/` Done -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/38927?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: I35c214fbe930c695a1475d8b4bc3dc44dff83eea Gerrit-Change-Number: 38927 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <[email protected]> Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Attention: fixeria <[email protected]> Gerrit-Attention: daniel <[email protected]> Gerrit-Comment-Date: Thu, 28 Nov 2024 15:59:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <[email protected]>
