lchan->s and integer overflow

[Holger Hans Peter Freyther]

On Tue, Mar 12, 2013 at 01:24:44PM +0100, Holger Hans Peter Freyther wrote:

Good Morning,

> that is what I was missing when browsing the code in gtik. I think
> there is a small glitch the other way around.
> 
> a.) We get criteria value == 0 through OML (for whatever reason)
> 
> +               btsb->radio_link_timeout = val[1]
> 
> b.) lchan_activate sets lchan->s == 0
> 
> +       lchan->s = btsb->radio_link_timeout;
> 
> c.) lchan->s -= 1
> 
> +                       /* count down radio link counter S */
> +                       lchan->s--;
> 
> d.) if (lchan->s == 0)
> 
> Will not be true as the lchan->s is -1. Is it intended that setting a
> '0' will disable the link failure checking?

the easiest way to avoid the problem of the number to leave the range
of 0 to radio_link_timeout is to use the OSMO_MAX and OSMO_MIN (now that
these macros are fixed).

Underflow:
lchan->s = OSMO_MAX(lchan->s - 1, 0);

Overflow:
lchan->s = OSMO_MIN(lchan->s + 2, btsb->radio_link_timeout);


E.g. the following code (I only compiled it):

diff --git a/src/osmo-bts-sysmo/l1_if.c b/src/osmo-bts-sysmo/l1_if.c
index df660c5..16375ef 100644
--- a/src/osmo-bts-sysmo/l1_if.c
+++ b/src/osmo-bts-sysmo/l1_if.c
@@ -684,7 +684,7 @@ static int handle_ph_data_ind(struct femtol1_hdl *fl1, GsmL1_PhDataInd_t *d
                /* process radio link timeout coniter S */
                if (data_ind->msgUnitParam.u8Size == 0) {
                        /* count down radio link counter S */
-                       lchan->s--;
+                       lchan->s = OSMO_MAX(lchan->s - 1, 0);
                        DEBUGP(DMEAS, "counting down radio link counter S=%d\n",
                                lchan->s);
                        if (lchan->s == 0)
@@ -694,9 +694,7 @@ static int handle_ph_data_ind(struct femtol1_hdl *fl1, GsmL1_PhDataInd_t *d
                }
                if (lchan->s < btsb->radio_link_timeout) {
                        /* count up radio link counter S */
-                       lchan->s += 2;
-                       if (lchan->s > btsb->radio_link_timeout)
-                               lchan->s = btsb->radio_link_timeout;
+                       lchan->s = OSMO_MIN(lchan->s + 2, btsb->radio_link_timeout);
                        DEBUGP(DMEAS, "counting up radio link counter S=%d\n",
                                lchan->s);
                }