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)) {
> const uint8_t *val = TLVP_VAL(&tp, NM_ATT_CONN_FAIL_CRIT);
> - 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 */
