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?