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: (4 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@1442 PS1, Line 1442: extern const struct value_string chan_count2_strs[]; > If you don't move them yourself, Pau will do this during his next refactoring ;) Ack https://gerrit.osmocom.org/c/osmo-bsc/+/25972/1/include/osmocom/bsc/gsm_data.h@1448 PS1, Line 1448: typedef unsigned int chan_counts_t [CHAN_COUNTS1_N][CHAN_COUNTS2_N][_GSM_LCHAN_MAX]; > lol, the linter says "NEW_TYPEDEFS: do not add new typedefs" […] I'd go for the struct with the multidimensional array inside it. This way it can be easily extended if needed for whatever reason. 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@1974 PS1, Line 1974: free_tchf = bts_counts[CHAN_COUNTS1_ALL][CHAN_COUNTS2_FREE][GSM_LCHAN_TCH_F]; > To me it all seems super obvious, could it be about improving readability? […] Yes please, a getter like you wrote, and with some minimal description like "Remember to call bts_counts_chans() or alike before using it. 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) > re "all logic in one file" -- who introduced the separation of bts and trx? I dimly remember that th […] As I said, I'm fine with keeping it either way, but if you keep it here, let's make it an operation on trx and have trx as first param, since the function is called "trx_count_lchans()". If you want to keep the ret function first, then it'd be clear naming it "count_lchans_trx" and moving it to the count specific file. This way one has some code which is consistent in some sort of object-oriented way. So ig a function starts with trx_*, then you know it's an operation on trx and the first param is a trx. -- 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-CC: fixeria <vyanitskiy at sysmocom.de> Gerrit-Comment-Date: Thu, 28 Oct 2021 11:47:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr at sysmocom.de> Comment-In-Reply-To: fixeria <vyanitskiy at sysmocom.de> Comment-In-Reply-To: pespin <pespin at sysmocom.de> Gerrit-MessageType: comment -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20211028/b4f0bea4/attachment.htm>