Pau Espin Pedrol has posted comments on this change. ( https://gerrit.osmocom.org/11789 )
Change subject: gsm: Deprecate buggy gsm_arfcn2band API and introduce gsm_arfcn2band_rc ...................................................................... Patch Set 1: > Sorry, but I don't think it makes sense to introduce a new symbol. > Why not to introduce something like: GSM_BAND_NONE or > GSM_BAND_INVAL (0x00)? > I already thought about this possibility first, but decided to do it this way finally. Because then all code using switch() statements on enum gsm_band will start failing and will break several repositories, and need to check for default value or that specific enum value in lots of places were really doesn't make sense since it should not happen (as in you expect a valid band). So really having to change lots of places which expect a valid band to fix a single place were unknown band can be returned makes no sense to me. > Moreover, you're changing the logic of old gsm_arfcn2band() > anyway... You are changing the logic of osmo_arfcn2band too by doing it the way you say, so no real change here. It's still not changing the API I'd argue, because I'd say passing an invalid arfcn was not contemplated for this API, since obviously makes no sense to return DCS1800 in that case. So this commit actually makes the API more concrete for some corner cases which used to be "undefined behaviour". -- To view, visit https://gerrit.osmocom.org/11789 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I780d452dcebce385469e32ef2fd844df6033393a Gerrit-Change-Number: 11789 Gerrit-PatchSet: 1 Gerrit-Owner: Pau Espin Pedrol <[email protected]> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Pau Espin Pedrol <[email protected]> Gerrit-Reviewer: Vadim Yanitskiy <[email protected]> Gerrit-Comment-Date: Fri, 16 Nov 2018 14:45:14 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No
