Attention is currently required from: osmith.

View Change

5 comments:

• File include/osmocom/bsc_nat/bsc_nat.h:

  • Patch Set #2, Line 26: BSC_NAT_CN = 0,

    you probably lack a prefix there for the enum. BSC_NAT is the domain, and CN and RAN the elements. BSC_NAT_NET_CN BSC_NAT_NET_RAN?

  • Patch Set #2, Line 48: uint32_t subscr_conn_id_next[2]; /* for CN, RAN */

    I'd simply have 2 variables here, looks easier to identify them in code, and I bet there's no real gain in having it as an array.

• File include/osmocom/bsc_nat/subscr_conn.h:

  • Patch Set #2, Line 29: uint32_t id[2]; /* conn_id for CN, RAN */

    same, there's problably no good reason to have this as an array. Otherwise one needs to look whether 0 is CN or RAN, etc.

• File src/osmo-bsc-nat/bsc.c:

  • Patch Set #2, Line 58: subscr_conn_free_by_bsc(bsc);

    This looks weird. I'd expect a subscr being passed here seeing the name of the function. Sounds more like the name should be "bsc_free_all_subscr_conn()" or bsc_free_subscr_conn_all()"
    Or perhaps into bsc_nat.c and have bsc_nat_free_subscr_conn_by_bsc().

• File src/osmo-bsc-nat/msc.c:

  • Patch Set #2, Line 77: subscr_conn_free_by_msc(msc);

    same: msc_free_subscr_conn_all()

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