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!