This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Harald Welte gerrit-no-reply at lists.osmocom.orgPatch Set 2: Code-Review-1 (11 comments) https://gerrit.osmocom.org/#/c/3195/2/include/openbsc/gsm_data.h File include/openbsc/gsm_data.h: Line 117: uint8_t classmark3[14]; classmark3 length gets extended every so often when new 3GPP releases are coming out. I think it might make sense to either extend it to 256 bytes here, or allocate it dynamically. In any case, not a blocker, as long as we properly deal with the fact that the subscriber-provided CM3 might be longer (we then simply truncate). Maybe a comment here. https://gerrit.osmocom.org/#/c/3195/2/include/openbsc/osmo_msc.h File include/openbsc/osmo_msc.h: Line 13: #define MSC_HLR_REMOTE_PORT_DEFAULT 2222 we should probably go for a more 'reasonable' port number closer to the other ports that we already use. also, is this documented in the wiki / manual yet? https://gerrit.osmocom.org/#/c/3195/2/src/libbsc/bsc_vty.c File src/libbsc/bsc_vty.c: Line 840: TODO: add in libvlr? a VLR by definition holds all of its state in volatile memory (i.e. RAM). So let's remove that bogus TODO here. https://gerrit.osmocom.org/#/c/3195/2/src/libcommon-cs/common_cs_vty.c File src/libcommon-cs/common_cs_vty.c: Line 232: vty_out(vty, "%% subscriber-keep-in-ram is currently not implemented%s", just drop it altogether? https://gerrit.osmocom.org/#/c/3195/2/src/libmsc/gsm_04_08.c File src/libmsc/gsm_04_08.c: Line 730: memcpy(conn->classmark.classmark2, classmark2, classmark2_len); we can overflow classmark2 buffer here, as we unconditionally use the user-provided length from the packet. We should truncate and log (with NOTICE level?) the fact that we had to truncate a too long value. Line 1112: lai.lac = 23; /* FIXME bts->location_area_code; */ I'm really not sure we can simply use hard-coded location area information here. What is this information used for later on down the road? Are we sure this is safe? If so, an explanation might be useful here. https://gerrit.osmocom.org/#/c/3195/2/src/libmsc/osmo_msc.c File src/libmsc/osmo_msc.c: Line 79: whitespace https://gerrit.osmocom.org/#/c/3195/2/src/libmsc/token_auth.c File src/libmsc/token_auth.c: Line 23: VLR: what do do with this? remove all of it. https://gerrit.osmocom.org/#/c/3195/2/src/libmsc/transaction.c File src/libmsc/transaction.c: Line 194: void trans_conn_closed(struct gsm_subscriber_connection *conn) all other functions have received nice doxygen documentation, not this one? ;) https://gerrit.osmocom.org/#/c/3195/2/src/osmo-nitb/bsc_hack.c File src/osmo-nitb/bsc_hack.c: Line 264: #if 0 remove https://gerrit.osmocom.org/#/c/3195/2/tests/mm_auth/mm_auth_test.c File tests/mm_auth/mm_auth_test.c: Line 13: #if 0 shouldn't the entire test case be dropped? -- To view, visit https://gerrit.osmocom.org/3195 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I639544a6cdda77a3aafc4e3446a55393f60e4050 Gerrit-PatchSet: 2 Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-HasComments: Yes