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 <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-CC: msuraev <[email protected]>
Gerrit-CC: neels <[email protected]>
Gerrit-Attention: pespin <[email protected]>
Gerrit-Attention: msuraev <[email protected]>
Gerrit-Comment-Date: Mon, 13 Feb 2023 03:15:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: msuraev <[email protected]>
Gerrit-MessageType: comment

Reply via email to