Attention is currently required from: laforge, lynxis lazus, pespin.

neels has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-iuh/+/38946?usp=email )

Change subject: iu_client: add ranap_iu_page_cs2/ranap_iu_page_ps2 ......................................................................

Patch Set 17: Code-Review+2

(6 comments)

File src/iu_client.c:

https://gerrit.osmocom.org/c/osmo-iuh/+/38946/comment/52c523b0_b47d8771?usp=... : PS13, Line 842: LOGL_ERROR

yes, but I think error is a little bit harsh, only when a client isn't reachable. […]

(the idea was to separate orthogonal changes into separate patches)

https://gerrit.osmocom.org/c/osmo-iuh/+/38946/comment/b4322398_bb340316?usp=... : PS13, Line 879: exact

I think this is possible for hnbs and not forbidden.

Done

https://gerrit.osmocom.org/c/osmo-iuh/+/38946/comment/ba3cbf23_0f893bfb?usp=... : PS13, Line 883: if (!osmo_lai_cmp(&entry->rai.lac, lai)) {

What do you mean? […]

nevermind if you don't care, just sharing the thought:

this could use the "early exit" or "return early" coding style, that reduces indented blocks. here it would be

llist_for_each_entry(rnc, &rnc_list, entry) { llist_for_each_entry(entry, &rnc->lac_rac_list, entry) { if (osmo_lai_cmp(&entry->rai.lac, lai)) continue;

rc = iu_tx_paging_cmd(&rnc->sccp_addr, imsi, tmsi, false, 0); if (rc > 0) { LOGPIU(LOGL_ERROR, "IuCS: Failed to tx Paging RNC %s for LAC %s for IMSI %s / TMSI %08x", osmo_rnc_id_name(&rnc->rnc_id), osmo_lai_name(lai), imsi, tmsi ? *tmsi : GSM_RESERVED_TMSI); } paged++; break; } }

https://gerrit.osmocom.org/c/osmo-iuh/+/38946/comment/91b196e8_d727dd66?usp=... : PS13, Line 893: }

No, we need to tx a page request to a RNC at most once. […]

Done

https://gerrit.osmocom.org/c/osmo-iuh/+/38946/comment/d7eacace_ce84ab9f?usp=... : PS13, Line 914: int ranap_iu_page_ps2(const char *imsi, const uint32_t *ptmsi, const struct osmo_routing_area_id *rai)

will add it to the header.

i'm not entirely sure of the actual reason why we're supposed to put api doc in the .c files. i think it might have to do with licensing -- like the .h files are just the index while the .c file is protected by the license, including the docs?

In practice, i always thought the docs should go in the .h, but i also found that i really enjoy a .h file to be short: in a few seconds i can glance across the entire API of functions. The long docs are just one ctags jump away.

I think conformance with osmocom style would suggest to put the docs in the .c file. but i won't block on it.

https://gerrit.osmocom.org/c/osmo-iuh/+/38946/comment/516ecd12_f6cb6e01?usp=... : PS13, Line 938: }

break is fine. See above.

Done