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/.
pespin gerrit-no-reply at lists.osmocom.orgpespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/25972 ) Change subject: refactor lchan counting ...................................................................... Patch Set 1: Code-Review-1 (8 comments) https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/include/osmocom/bsc/gsm_data.h File include/osmocom/bsc/gsm_data.h: https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/include/osmocom/bsc/gsm_data.h@1439 PS1, Line 1439: CHAN_COUNTS2_N is this simply to flag the end? please document so, since it's a bit confusing as this is an enum enumerating types of counting ;) https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/include/osmocom/bsc/gsm_data.h@1442 PS1, Line 1442: extern const struct value_string chan_count2_strs[]; Lots of new infra, sounds like all this can be moved to a new header file. https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/src/osmo-bsc/abis_rsl.c File src/osmo-bsc/abis_rsl.c: https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/src/osmo-bsc/abis_rsl.c@1961 PS1, Line 1961: chan_counts_t bts_counts; we don't tend to use *_t types in osmocom (see kernel programming style guide). If it's a struct, describe it as a struct. https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/src/osmo-bsc/abis_rsl.c@1974 PS1, Line 1974: free_tchf = bts_counts[CHAN_COUNTS1_ALL][CHAN_COUNTS2_FREE][GSM_LCHAN_TCH_F]; I'd rather prefer getters here, this way the array can be kept internal to its own module and no need for other modules to know how counting ocurrs. Moreover, some comment here describing what you fetch here with all those enums is welcome. https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/src/osmo-bsc/bts.c File src/osmo-bsc/bts.c: https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/src/osmo-bsc/bts.c@728 PS1, Line 728: chan_counts_t trx_counts; same, no *_t types, only for basic type typedefs. https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/src/osmo-bsc/bts_trx.c File src/osmo-bsc/bts_trx.c: https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/src/osmo-bsc/bts_trx.c@327 PS1, Line 327: void trx_count_lchans(chan_counts_t ret, const struct gsm_bts_trx *trx) All this could go into a new counts.c file, to have all this logic in one place. Alternatively, pass trx as first param as in object-oriented, since also the function starts with trx_* https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/src/osmo-bsc/gsm_data.c File src/osmo-bsc/gsm_data.c: https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/src/osmo-bsc/gsm_data.c@650 PS1, Line 650: * TODO: include possible VAMOS secondary lchans? */ what about this TODO? https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/src/osmo-bsc/handover_decision_2.c File src/osmo-bsc/handover_decision_2.c: https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/src/osmo-bsc/handover_decision_2.c@997 PS1, Line 997: c->current.free_tchf = bts_counts[CHAN_COUNTS1_ALL][CHAN_COUNTS2_FREE][GSM_LCHAN_TCH_F]; same, use getters (be it static inline or whatever) -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/25972 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I2fb48c549186db812b1e9d6b735a92e80f27b8d3 Gerrit-Change-Number: 25972 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin at sysmocom.de> Gerrit-Comment-Date: Wed, 27 Oct 2021 12:39:10 +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/20211027/006d0800/attachment.htm>