Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476 )
Change subject: Support forwarding messages with multiple BSCs ...................................................................... Patch Set 2: (5 comments) File include/osmocom/bsc_nat/bsc_nat.h: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/4c3504c2_55095343 PS2, 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? https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/680b2c88_b95f3d9d PS2, 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: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/c4120d16_cf2b1bdb PS2, 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: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/6621e8c6_83fcf79d PS2, 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: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/a683dc52_5664cc5e PS2, Line 77: subscr_conn_free_by_msc(msc); same: msc_free_subscr_conn_all() -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc-nat Gerrit-Branch: master Gerrit-Change-Id: I1556aa665fbb0a97507f98794e74820731fa6935 Gerrit-Change-Number: 27476 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <[email protected]> Gerrit-Attention: osmith <[email protected]> Gerrit-Comment-Date: Mon, 14 Mar 2022 10:23:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
