On Thu, Mar 14, 2013 at 07:47:25AM +0100, jolly wrote:
Holger Hans Peter Freyther wrote:
Hi,
this must be frustrating for you, but it really is for me as well. I take absolutely no joy in pointing out these issues. I would prefer to work on my code but it is something that will bite us in a commercial setup so I a have to be the PITA here.
/* 9.4.14 Connection Failure Criterion */ if (TLVP_PRESENT(&tp, NM_ATT_CONN_FAIL_CRIT) &&
(TLVP_LEN(&tp, NM_ATT_CONN_FAIL_CRIT) >= 2) &&*TLVP_VAL(&tp, NM_ATT_CONN_FAIL_CRIT) == 0x01) {
(TLVP_LEN(&tp, NM_ATT_CONN_FAIL_CRIT) >= 2)) {
btsb->radio_link_timeout = val[1];
if (val[0] == 0x01 && val[1] >= 4)btsb->radio_link_timeout = val[1];
Here you point out that the range of 4 to UINT8_MAX is coming from the specification. Which is very good. The way it is done is a violation of the principle of least surprise though. If a BSC sends the value '2' it doesn't make sense that '32' is used. The configurator/implementor certainly wanted to have a low value.
IMHO the right thing to do is to NACK the set bts attributes with a descriptive error message. The same goes for the conn fail crit being present but not of the type we support.
holger
PS:
the lchan->s handling is fine. The switch/case has grown to a state I would move the link failure handling to a new helper function though, this way one could even write a unit test for the handling...
switch (data_ind->sapi) { case GsmL1_Sapi_Sacch:
/* process radio link timeout coniter S */
/* process radio link timeout counter S */