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/.
Pau Espin Pedrol gerrit-no-reply at lists.osmocom.orgPatch Set 1: (5 comments) https://gerrit.osmocom.org/#/c/7723/1/include/osmocom/bsc/acc_ramp.h File include/osmocom/bsc/acc_ramp.h: Line 145: void acc_ramp_disable(struct acc_ramp *acc_ramp); I think it makes more sense to have it as one function, no need for two: acc_ramp_enabled_set(acc_ramp, bool enabled) This way it's easier to grep in code with 1 token to see where it is enabled or not,but not really required of course. In any case, the enable/disable could then be implemented as defines or static inline on top of acc_ramp_enabled_set, but I still prefer having only 1 function and use it everywhere. Even better, it can be set as a static inline as I see it's already done for similar stuff in here. https://gerrit.osmocom.org/#/c/7723/1/src/libbsc/acc_ramp.c File src/libbsc/acc_ramp.c: Line 34: static bool acc_is_enabled(struct gsm_bts *bts, unsigned int acc) Not directly related to this patch but: Use of same terminology (acc enabled/disabled) for different structs containing different information (permanent vs temporary due to ramping activation) is quite confusing. Line 120: if (acc_ramp->step_size == ACC_RAMP_STEP_SIZE_MAX) { I think we can drop this if and its contents completely in a follow up patch. It is not needed, it's handled fine by the generic code path, so no need to add code complexity for a really corner case scenario (since usually in that case you would actually just disable ramping). https://gerrit.osmocom.org/#/c/7723/1/src/libbsc/bsc_vty.c File src/libbsc/bsc_vty.c: Line 1918: acc_ramp_init(&bts->acc_ramp, bts); I think it really makes sense to move _init inside gsm_bts_alloc_register now, as it's only used at this time. Line 3278: acc_ramp_enable(&bts->acc_ramp); You want to dothe same in here (abort + set_system_infos) as you do in no_acc_ramping below. Imagine for instance if a vty cfg file contains "access-control-class-ramping" twice, or if someone enters "access-control-class-ramping" in the vty while a ramping is still in progress. -- To view, visit https://gerrit.osmocom.org/7723 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia25bff85d9e5c277da76bffa11d31972e9fdc323 Gerrit-PatchSet: 1 Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de> Gerrit-HasComments: Yes