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=email : 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=email : 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=email : 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=email : 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=email : 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=email : PS13, Line 938: } > break is fine. See above. Done -- To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/38946?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-iuh Gerrit-Branch: master Gerrit-Change-Id: I1f07e96642737160d387de3e4c3f71d288d356dd Gerrit-Change-Number: 38946 Gerrit-PatchSet: 17 Gerrit-Owner: lynxis lazus <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: neels <[email protected]> Gerrit-Reviewer: pespin <[email protected]> Gerrit-Attention: laforge <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Attention: lynxis lazus <[email protected]> Gerrit-Comment-Date: Thu, 20 Feb 2025 21:21:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: neels <[email protected]> Comment-In-Reply-To: lynxis lazus <[email protected]>
