Attention is currently required from: pespin, fixeria, dexter. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31578 ) Change subject: pcu_sock: use struct to transfer IMMEDIATE ASSIGNMENT for PCH ...................................................................... Patch Set 8: (4 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-bsc/+/31578/comment/af14cb7f_934b30d7 PS8, Line 10: "direct TLLI" method, the TLLI (and the paging group) is premended to probably "prepended" File src/osmo-bsc/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bsc/+/31578/comment/b899a834_0a6023a9 PS8, Line 579: pch_dt->tlli, pch_dt->imsi, pag_grp); you keep saying that, but single-tab indent is a perfectly fine alternative to indenting lined up with the opening brace, and I keep saying that. https://gerrit.osmocom.org/c/osmo-bsc/+/31578/comment/218fd7c8_e18cce71 PS8, Line 581: msg = msgb_alloc(sizeof(pch_dt->data), "pcu_pch"); this is just moving code from a previous patch, but in fact this msgb is leaked! It should have been freed in all code paths, rc or !rc. (There seems to be no existing msgb to remove the header of, pch_dt is not a msgb) But since you don't pass msg as arg anywhere, but only pass its data and len, might as well just pass the pch_dt->data and its size to rsl_ericsson_imm_assign_cmd() below. That skips the unnecessary msgb allocation and hence there can be no msgb leak. And since this problem exists before this patch, rather submit one separate patch before this, to fix the msgb thing on its own, so that this patch only shows the intended functional change. https://gerrit.osmocom.org/c/osmo-bsc/+/31578/comment/ab4c0b6b_e8bc311c PS8, Line 595: msg->len, msg->data ...here just pass the pch_dt->data -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31578 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Id6acbd243adf26169e5e8319dd66bb68dd6a3c22 Gerrit-Change-Number: 31578 Gerrit-PatchSet: 8 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Sat, 04 Mar 2023 00:23:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment