Attention is currently required from: pespin, msuraev.
neels has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmo-gprs/+/31286 )
Change subject: rlcmac: Introduce DL TBF creation through PCH ImmAss
......................................................................
Patch Set 5:
(3 comments)
File src/rlcmac/rlcmac.c:
https://gerrit.osmocom.org/c/libosmo-gprs/+/31286/comment/d0151c70_6519b4f4
PS5, Line 278: OSMO_ASSERT(bv);
I think so far common practice was to log memory
allocation errors and shutdown gracefully. […]
when mem alloc fails, logging cannot
possibly work. we do in fact assert on alloc success very often. i agree with this
assert.
https://gerrit.osmocom.org/c/libosmo-gprs/+/31286/comment/552b7fd8_e520b6c1
PS5, Line 291: LOGRLCMAC(LOGL_NOTICE, "TODO: handle decoded dl ctrl
block!\n");
hmm, a TODO in the logs? maybe a #warning pragma instead?
File src/rlcmac/tbf_dl.c:
https://gerrit.osmocom.org/c/libosmo-gprs/+/31286/comment/b45fbc9a_ecc23765
PS5, Line 39: // goto err_tbf_destruct;
i can understand that some code will come in only later, but you do add an awful lot of
these temporarily commented things. Seems that a lot of them will end up surviving
eternally and should rather be dropped from patches being merged; let's not let this
become a habit, who will tidy up all of them?
--
To view, visit
https://gerrit.osmocom.org/c/libosmo-gprs/+/31286
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: I7f98e3456ef35d80becdad3481afeb771457b0ef
Gerrit-Change-Number: 31286
Gerrit-PatchSet: 5
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: msuraev <msuraev(a)sysmocom.de>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: msuraev <msuraev(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 13 Feb 2023 03:15:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de>
Gerrit-MessageType: comment