[PATCH] gsm_04_08: Use osmo_assert for transt->conn and conn only in case of paging succeeded
setup_trig_pag_evt function can receive parameter conn = NULL, if T3113 expires. --- openbsc/src/libmsc/gsm_04_08.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index 5609602..d45ae08 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -1389,13 +1389,12 @@ static int setup_trig_pag_evt(unsigned int hooknum, unsigned int event, struct gsm_subscriber_connection *conn = _conn; struct gsm_trans *transt = _transt;
- OSMO_ASSERT(!transt->conn); - OSMO_ASSERT(conn); - /* check all tranactions (without lchan) for subscriber */ switch (event) { case GSM_PAGING_SUCCEEDED: DEBUGP(DCC, "Paging subscr %s succeeded!\n", transt->subscr->extension); + OSMO_ASSERT(!transt->conn); + OSMO_ASSERT(conn); /* Assign lchan */ transt->conn = conn; /* send SETUP request to called party */
On 26 May 2015, at 12:53, Ivan Kluchnikov kluchnikovi@gmail.com wrote:
HI,
setup_trig_pag_evt function can receive parameter conn = NULL, if T3113 expires.
ah thank you very much for the patch and you are obviously right that in case the paging times out there will be no subscriber connection.
openbsc/src/libmsc/gsm_04_08.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/openbsc/src/libmsc/gsm_04_08.c b/openbsc/src/libmsc/gsm_04_08.c index 5609602..d45ae08 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -1389,13 +1389,12 @@ static int setup_trig_pag_evt(unsigned int hooknum, unsigned int event, struct gsm_subscriber_connection *conn = _conn; struct gsm_trans *transt = _transt;
- OSMO_ASSERT(!transt->conn);
Is there any reason to move this assert as well? setup_trig_pag_evt should be called exactly once right? And there should be no pre-existing subscriber connection? Do you have time to re-test with just moving one of the two asserts?
thank you
holger
Hi Holger,
Yes, you are right, I retested this patch and it is enough to move only OSMO_ASSERT(conn). I updated patch for this issue.
2015-05-27 13:09 GMT+03:00 Holger Freyther holger@freyther.de:
On 26 May 2015, at 12:53, Ivan Kluchnikov kluchnikovi@gmail.com wrote:
HI,
setup_trig_pag_evt function can receive parameter conn = NULL, if T3113
expires.
ah thank you very much for the patch and you are obviously right that in case the paging times out there will be no subscriber connection.
openbsc/src/libmsc/gsm_04_08.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/openbsc/src/libmsc/gsm_04_08.c
b/openbsc/src/libmsc/gsm_04_08.c
index 5609602..d45ae08 100644 --- a/openbsc/src/libmsc/gsm_04_08.c +++ b/openbsc/src/libmsc/gsm_04_08.c @@ -1389,13 +1389,12 @@ static int setup_trig_pag_evt(unsigned int
hooknum, unsigned int event,
struct gsm_subscriber_connection *conn = _conn; struct gsm_trans *transt = _transt;
OSMO_ASSERT(!transt->conn);Is there any reason to move this assert as well? setup_trig_pag_evt should be called exactly once right? And there should be no pre-existing subscriber connection? Do you have time to re-test with just moving one of the two asserts?
thank you
holger
On 29 May 2015, at 10:56, Ivan Kluchnikov Ivan.Kluchnikov@fairwaves.ru wrote:
Hi Holger,
Yes, you are right, I retested this patch and it is enough to move only OSMO_ASSERT(conn). I updated patch for this issue.
thanks, I cherry-picked it from your branch.
holger
-
Holger Freyther -
Ivan Kluchnikov -
Ivan Kluchnikov