Attention is currently required from: laforge, lynxis lazus.

Patch set 13:Code-Review +1

View Change

14 comments:

• Patchset:

  • Patch Set #13:

    in the end, after CR, i notice that this is the iu_client.c code -- which osmo-msc does not use anymore IIRC, meaning that all of the _cs code here is dead? So it might make sense to drop iu_client and copy what osmo-sgsn needs into osmo-sgsn.git. iu_client was an early attempt by me to abstract some common parts between SGSN and MSC, but ended up making more problems than helping. At some point osmo-msc (years ago) started its own copy of this code, to move independently.

    we can merge this if you like, of course.

• Commit Message:

  • Patch Set #13, Line 9: new

    hehe, the slightly newER =)

• File src/iu_client.c:

  • Patch Set #13, Line 842: LOGL_ERROR

    (marking: this was ERROR)

  • Patch Set #13, Line 869: int ranap_iu_page_cs2(const char *imsi, const uint32_t *tmsi, const struct osmo_location_area_id *lai)

    it would be great to have an API doc explaining how version 2 behaves differently

  • Patch Set #13, Line 879: exact

    "exactly one"

    -- but is that overall exactly one paging to exactly one RNC?
    the code looks more like one per RNC that matches the lai ..

    am i even making sense, can multiple RNC match the same LAI = PLMN+LAC?

  • Patch Set #13, Line 883: if (!osmo_lai_cmp(&entry->rai.lac, lai)) {

    (early exit: 'if (osmo_lai_cmp(..)) continue;')

  • Patch Set #13, Line 893: }

    if it's exactly one, then it seems that this loop could also get an 'if (paged) break;' ?

  • Patch Set #13, Line 900: %d RN

    (then again this looks like multiple matches are possible)

    please use snprintf() with sizeof(log_msg)

  • Patch Set #13, Line 909: LOGPIUC(log_level, ", for IMSI %s\n", imsi);

    please avoid LOGPC, because for gsm_tap_log it rips apart log lines to multiple gsmtap packages. so let's rather add ugly 'tmsi ? "TMSI" : "IMSI"' args, and a '\n', above

  • Patch Set #13, Line 914: int ranap_iu_page_ps2(const char *imsi, const uint32_t *ptmsi, const struct osmo_routing_area_id *rai)

    api doc

  • Patch Set #13, Line 938: }

    if (paged) break?

  • Patch Set #13, Line 943: LOGL_INFO

    this previously was LOGL_ERROR and the _cs2 also does ERROR.
    If it should change, explain in commit log and/or move the change to separate commit?

  • Patch Set #13, Line 945: sprintf(log_msg, "Paged %d RNCs", paged);

    snprintf()

  • Patch Set #13, Line 954: LOGPIUC(log_level, ", for IMSI %s\n", imsi);

    avoid LOGPC

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