[M] Change in osmo-iuh[master]: iu_client: add ranap_iu_page_cs2/ranap_iu_page_ps2 - gerrit-log

20 Feb 2025

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

-- 
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 &lt;lynxis(a)fe80.eu&gt;
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel &lt;dwillmann(a)sysmocom.de&gt;
Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt;
Gerrit-Reviewer: neels &lt;nhofmeyr(a)sysmocom.de&gt;
Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt;
Gerrit-Attention: laforge &lt;laforge(a)osmocom.org&gt;
Gerrit-Attention: pespin &lt;pespin(a)sysmocom.de&gt;
Gerrit-Attention: lynxis lazus &lt;lynxis(a)fe80.eu&gt;
Gerrit-Comment-Date: Thu, 20 Feb 2025 21:21:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: neels &lt;nhofmeyr(a)sysmocom.de&gt;
Comment-In-Reply-To: lynxis lazus &lt;lynxis(a)fe80.eu&gt;