Attention is currently required from: pespin. msuraev 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: (4 comments) File include/osmocom/gprs/rlcmac/tbf_dl.h: https://gerrit.osmocom.org/c/libosmo-gprs/+/31286/comment/b6643bee_c4015a4c PS5, Line 9: //#include <osmocom/gprs/rlcmac/tbf_dl_fsm.h> What's the point of commented include? Just drop it if it's unused. https://gerrit.osmocom.org/c/libosmo-gprs/+/31286/comment/d277c586_806a9aad PS5, Line 16: //struct gprs_rlcmac_tbf_dl_fsm_ctx state_fsm; Same here. File src/rlcmac/rlcmac.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/31286/comment/a3ea83e9_a35ef15f PS5, Line 278: OSMO_ASSERT(bv); I think so far common practice was to log memory allocation errors and shutdown gracefully. Why the use of assert in here? File src/rlcmac/tbf_dl.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/31286/comment/3aeaa564_9eef1242 PS5, Line 37: //rc = gprs_rlcmac_tbf_dl_fsm_constructor(dl_tbf); That looks odd. Is this an artifact from patch split? -- 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 &lt;pespin(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-CC: msuraev &lt;msuraev(a)sysmocom.de&gt; Gerrit-Attention: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Comment-Date: Sun, 12 Feb 2023 19:43:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment