Attention is currently required from: laforge, pespin, lynxis lazus. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hlr/+/32511 )
Change subject: hlr: use talloc for memory allocation in osmo_gsup_create_insert_subscriber_data_msg ...................................................................... Patch Set 3: Code-Review-1 (5 comments) File include/osmocom/hlr/gsup_server.h: https://gerrit.osmocom.org/c/osmo-hlr/+/32511/comment/1cd87028_0f269f13 PS3, Line 71: osmo_gsup_create_insert_subscriber_data_msg > this looks like a public API function of libosmo-gsup to me? In that case, > we cannot simply break A […] Looks like it is, although only osmo-hlr seem to be using it. I did `./gits do grep osmo_gsup_create_insert_subscriber_data_msg` and could not find any other osmo-projects using it. But still, I would avoid breaking public API even if none of our projects are using it. File src/gsup_server.c: https://gerrit.osmocom.org/c/osmo-hlr/+/32511/comment/6c61462d_d1ca4b91 PS3, Line 455: void *talloc_ctx) > because it's not guaranteed that gsup is a talloc allocated buffed. Agreeing with @lynxis here. File src/hlr.c: https://gerrit.osmocom.org/c/osmo-hlr/+/32511/comment/09aec568_aa6cc162 PS3, Line 133: sub_upd_ctx Let's better use `OTC_SELECT` and avoid `goto`s. If you don't like this idea, then maybe allocate this context outside of the loop and then free? Is there really a need to have a dedicated context for each iteration? File src/lu_fsm.c: https://gerrit.osmocom.org/c/osmo-hlr/+/32511/comment/7c3c16ba_cb978664 PS3, Line 249: lu_isr_ctx I suggest simply using `OTC_SELECT` here. https://gerrit.osmocom.org/c/osmo-hlr/+/32511/comment/facc7931_1ac18ee0 PS3, Line 251: return memleak! -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/32511 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I00b5c2dfadcf6e0740e93b4c3292d2654d22e80c Gerrit-Change-Number: 32511 Gerrit-PatchSet: 3 Gerrit-Owner: lynxis lazus <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-CC: laforge <[email protected]> Gerrit-CC: msuraev <[email protected]> Gerrit-CC: pespin <[email protected]> Gerrit-Attention: laforge <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Attention: lynxis lazus <[email protected]> Gerrit-Comment-Date: Sun, 30 Apr 2023 18:03:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: laforge <[email protected]> Comment-In-Reply-To: pespin <[email protected]> Comment-In-Reply-To: lynxis lazus <[email protected]> Gerrit-MessageType: comment
