Patch Set 1: Code-Review-1

File src/libbsc/acc_ramp.c:

Line 162:       struct gsm_nm_state *new_state = nsd->new_state;
You are deferring an expected nm_statechg_signal_data pointer before knowing if 
that's the correct signal (check for S_NM_STATECHG_ADM done below).

Line 166:       if (subsys != SS_NM)
Is this really needed? we are only attaching the cb to the SS_NM subsystem 

either move to an ASSERT or drop it (preferred imho).

Line 194:               break;
What about NM_STATE_SHUTDOWN and NM_STATE_NULL? Would be nice at least adding 
an explicit case for NM_STATE_SHUTDOWN, and maybe ASSERT on default.
File src/libbsc/bsc_vty.c:

Line 3280:      /* ACC ramping takes effect when the BTS reconnects or RF 
administrative state changes to 'unlocked'. */
You can take the chance to improve the "BTS reconnects" part by adding 
reference to RSL link.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I4124f1da3dadec003de45c1da8435506ee8f0a34
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling <>
Gerrit-Reviewer: Harald Welte <>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Pau Espin Pedrol <>
Gerrit-HasComments: Yes

Reply via email to