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… :
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… :
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… :
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… :
PS1, Line 944: uint8_t *data = msgb_l2(msg);
`const`?
Done
https://gerrit.osmocom.org/c/libosmo-abis/+/38927/comment/e67b92cd_7ef2b5a0… :
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… :
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 <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 28 Nov 2024 15:59:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>