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 <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 14 Mar 2022 10:23:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment