Patch Set 1: (2 comments)
I am not sure if a cleaner solution would be to fold all of this into one value. Like "early-classmark-allowed (2g-only|2g-3g|forbiden)" and a parser that accepts "allowed" as "2g-and-3g" to be compatible to old config files. What do you think? Am I missing something? https://gerrit.osmocom.org/#/c/5021/1/src/libbsc/bsc_vty.c File src/libbsc/bsc_vty.c: PS1, Line 281: vty_out(vty, "Early Classmark Sending: %s%s", : bts->early_classmark_allowed ? "allowed" : "forbidden", : VTY_NEWLINE); : vty_out(vty, "3G Early Classmark Sending: %s%s", : bts->early_classmark_allowed_3g ? "allowed" : "forbidden", : VTY_NEWLINE); might be nicer to have this in a single line with "Early Classmark Sending Allowed for: 2G, 3G" https://gerrit.osmocom.org/#/c/5021/1/src/libbsc/system_information.c File src/libbsc/system_information.c: PS1, Line 875: s are you sure about the logic? "restrict 3G" sounds like "restrict the use of Early CM Sending for 3G, i.e. *not* have Early CM for 3G. The "early_classmark_allowed" is exactly the opposite. Also, I don't like assigning a boolean value to an uint8_t, it looks a bit unclean. What's the problem with sticking either to bool or to the other? -- To view, visit https://gerrit.osmocom.org/5021 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic1afe071038a3bb5871d7ff40f665c8644f801ec Gerrit-PatchSet: 1 Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-HasComments: Yes
