Patch Set 3: Code-Review-1

File src/libbsc/acc_ramp.c:

Line 182:                       if (trx_is_usable(trx)) /* cross-check with 
operational state */
As discussed a few mins ago, when coming from S_NM_STATECHG_OPER signal in 
abis_nm_rx_statechg_rep, it can be that both operational and administrative 
status change at the same time (same call to this function).

Case: admin LOCKED->UNLOCKED + oper DISABLED->ENABLED, we are not enabling 
ramping here because trx_is_usable() probably checks the current/old status 
which is still DISABLED.

Line 190:                       break;
we can probably removed this break to print an error too, unless NULL is really 
a valid state from the spec point of view (one to which we can change to).

Line 201:                       if (trx->mo.nm_state.administrative == 
Again, we need to check after the new one as it could have be different than 
the old/current one.

Line 208:                       break;
same, remove break?

Line 222:       if (trigger_ramping && 
do we really need this osmo_timer_pending check here? This should never happen 
right? better move it inside the if() and add an OSMO_ASSERT() with it.

To view, visit
To unsubscribe, visit

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

Reply via email to