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/.
fixeria gerrit-no-reply at lists.osmocom.orgfixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/20990 ) Change subject: pcuif: Improve BTS-supported CS/MCS handling ...................................................................... Patch Set 1: Code-Review-1 (7 comments) https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/bts.h File src/bts.h: https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/bts.h@113 PS1, Line 113: bool cs_supported[4]; Why not bit-masks? https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/gprs_bssgp_pcu.cpp File src/gprs_bssgp_pcu.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/gprs_bssgp_pcu.cpp@746 PS1, Line 746: sizeof I would not make such assumptions that sizeof(bool) is always 1; it's safer to ARRAY_SIZE here. But using a bit-mask is even better, as I already pointed in another comment. https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/gprs_bssgp_pcu.cpp@749 PS1, Line 749: break Also, what if none of the MCS variants is supported? Would we leave 'num' uninitialized? https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/gprs_bssgp_pcu.cpp@769 PS1, Line 769: break Same here, but in this case it's less likely given that CS-1 is mandatory AFAIR. https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/pcu_l1_if.cpp File src/pcu_l1_if.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/pcu_l1_if.cpp@619 PS1, Line 619: two tabs? why? https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/pcu_l1_if.cpp@621 PS1, Line 621: any_cs_supported What is the motivation here? If the BTS indicates that none is supported, then it's a bug. https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/pcu_l1_if.cpp@625 PS1, Line 625: if (!any_cs_supported) /* We need at least 1 CS, let's enable CS1 */ What is the motivation here? If the BTS indicates that none is supported, then it's a bug. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/20990 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ie8f0215ba17da1e545e98bec9325c02f1e8efaea Gerrit-Change-Number: 20990 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de> Gerrit-Comment-Date: Sat, 31 Oct 2020 12:42:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20201031/32b96401/attachment.htm>