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

Patch set 17:Code-Review +2

View Change

6 comments:

• File src/iu_client.c:

  • Patch Set #13, 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)

  • Patch Set #13, Line 879: exact

    I think this is possible for hnbs and not forbidden.

    Done

  • Patch Set #13, 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;
    		}
    	}

  • Patch Set #13, Line 893: }

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

    Done

  • Patch Set #13, 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.

  • Patch Set #13, Line 938: }

    break is fine. See above.

    Done

To view, visit change 38946. To unsubscribe, or for help writing mail filters, visit settings.