GSCON: call api of a_reset.c with msc object directly

The API of a_reset.c is currently called with a pointer to struct
reset_ctx. This puts the responsibility of checking the presence of
msc->a.reset_fsm to the caller. It would be much more effective if the
caller would check if msc->a.reset_fsm before dereferencing it.

This also fixes at least one segfault that ocurrs when gscon_timer_cb()
is called but no sccp connection is present yet. Therefore the pointer
to bsc_msc_data would not be populated. This is now detected by
a_reset.c itsself.

- minor code cleanups
- call a_reset.c functions with msc (struct bsc_msc_data)

Change-Id: I0802aaadf0af4e58e41c98999e8c6823838adb61
Related: OS#3447

6 files changed, 54 insertions, 39 deletions

diff --git a/include/osmocom/bsc/a_reset.h b/include/osmocom/bsc/a_reset.h
index 6b6ce814a..a09972e18 100644
--- a/include/osmocom/bsc/a_reset.h
+++ b/include/osmocom/bsc/a_reset.h
@@ -20,17 +20,19 @@
 
 #pragma once
 
+struct bsc_msc_data;
+
 /* Create and start state machine which handles the reset/reset-ack procedure */
-struct osmo_fsm_inst *a_reset_alloc(void *ctx, const char *name, void *cb, void *priv);
+void a_reset_alloc(struct bsc_msc_data *msc, const char *name, void *cb);
 
 /* Confirm that we sucessfully received a reset acknowlege message */
-void a_reset_ack_confirm(struct osmo_fsm_inst *reset_fsm);
+void a_reset_ack_confirm(struct bsc_msc_data *msc);
 
 /* Report a failed connection */
-void a_reset_conn_fail(struct osmo_fsm_inst *reset_fsm);
+void a_reset_conn_fail(struct bsc_msc_data *msc);
 
 /* Report a successful connection */
-void a_reset_conn_success(struct osmo_fsm_inst *reset_fsm);
+void a_reset_conn_success(struct bsc_msc_data *msc);
 
 /* Check if we have a connection to a specified msc */
-bool a_reset_conn_ready(struct osmo_fsm_inst *reset_fsm);
+bool a_reset_conn_ready(struct bsc_msc_data *msc);
diff --git a/src/osmo-bsc/a_reset.c b/src/osmo-bsc/a_reset.c
index b8f8c8cbd..3c2114219 100644
--- a/src/osmo-bsc/a_reset.c
+++ b/src/osmo-bsc/a_reset.c
@@ -137,68 +137,83 @@ static struct osmo_fsm fsm = {
 };
 
 /* Create and start state machine which handles the reset/reset-ack procedure */
-struct osmo_fsm_inst *a_reset_alloc(void *ctx, const char *name, void *cb, void *priv)
+void a_reset_alloc(struct bsc_msc_data *msc, const char *name, void *cb)
 {
-	OSMO_ASSERT(name);
-
 	struct reset_ctx *reset_ctx;
 	struct osmo_fsm_inst *reset_fsm;
 
+	OSMO_ASSERT(msc);
+	OSMO_ASSERT(name);
+	OSMO_ASSERT(cb);
+
+	/* There must not be any double allocation! */
+	OSMO_ASSERT(msc->a.reset_fsm == NULL);
+
 	/* Register the fsm description (if not already done) */
 	if (osmo_fsm_find_by_name(fsm.name) != &fsm)
 		osmo_fsm_register(&fsm);
 
 	/* Allocate and configure a new fsm instance */
-	reset_ctx = talloc_zero(ctx, struct reset_ctx);
+	reset_ctx = talloc_zero(msc, struct reset_ctx);
 	OSMO_ASSERT(reset_ctx);
-	reset_ctx->priv = priv;
+	reset_ctx->priv = msc;
 	reset_ctx->cb = cb;
 	reset_ctx->conn_loss_counter = 0;
-	reset_fsm = osmo_fsm_inst_alloc(&fsm, ctx, reset_ctx, LOGL_DEBUG, name);
+	reset_fsm = osmo_fsm_inst_alloc(&fsm, msc, reset_ctx, LOGL_DEBUG, name);
 	OSMO_ASSERT(reset_fsm);
 
 	/* kick off reset-ack sending mechanism */
 	osmo_fsm_inst_state_chg(reset_fsm, ST_DISC, RESET_RESEND_INTERVAL, RESET_RESEND_TIMER_NO);
 
-	return reset_fsm;
+	msc->a.reset_fsm = reset_fsm;
 }
 
 /* Confirm that we sucessfully received a reset acknowlege message */
-void a_reset_ack_confirm(struct osmo_fsm_inst *reset_fsm)
+void a_reset_ack_confirm(struct bsc_msc_data *msc)
 {
-	OSMO_ASSERT(reset_fsm);
-	osmo_fsm_inst_dispatch(reset_fsm, EV_RESET_ACK, NULL);
+	if (!msc)
+		return;
+
+	if (!msc->a.reset_fsm)
+		return;
+
+	osmo_fsm_inst_dispatch(msc->a.reset_fsm, EV_RESET_ACK, NULL);
 }
 
 /* Report a failed connection */
-void a_reset_conn_fail(struct osmo_fsm_inst *reset_fsm)
+void a_reset_conn_fail(struct bsc_msc_data *msc)
 {
-	/* If no reset context is supplied, just drop the info */
-	if (!reset_fsm)
+	if (!msc)
+		return;
+
+	if (!msc->a.reset_fsm)
 		return;
 
-	osmo_fsm_inst_dispatch(reset_fsm, EV_N_DISCONNECT, NULL);
+	osmo_fsm_inst_dispatch(msc->a.reset_fsm, EV_N_DISCONNECT, NULL);
 }
 
 /* Report a successful connection */
-void a_reset_conn_success(struct osmo_fsm_inst *reset_fsm)
+void a_reset_conn_success(struct bsc_msc_data *msc)
 {
-	/* If no reset context is supplied, just drop the info */
-	if (!reset_fsm)
+	if (!msc)
 		return;
 
-	osmo_fsm_inst_dispatch(reset_fsm, EV_N_CONNECT, NULL);
+	if (!msc->a.reset_fsm)
+		return;
+
+	osmo_fsm_inst_dispatch(msc->a.reset_fsm, EV_N_CONNECT, NULL);
 }
 
 /* Check if we have a connection to a specified msc */
-bool a_reset_conn_ready(struct osmo_fsm_inst *reset_fsm)
+bool a_reset_conn_ready(struct bsc_msc_data *msc)
 {
-	/* If no reset context is supplied, we assume that
-	 * the connection can't be ready! */
-	if (!reset_fsm)
+	if (!msc)
+		return false;
+
+	if (!msc->a.reset_fsm)
 		return false;
 
-	if (reset_fsm->state == ST_CONN)
+	if (msc->a.reset_fsm->state == ST_CONN)
 		return true;
 
 	return false;
diff --git a/src/osmo-bsc/bsc_subscr_conn_fsm.c b/src/osmo-bsc/bsc_subscr_conn_fsm.c
index 81fe9f6cc..32375d47f 100644
--- a/src/osmo-bsc/bsc_subscr_conn_fsm.c
+++ b/src/osmo-bsc/bsc_subscr_conn_fsm.c
@@ -771,7 +771,7 @@ static int gscon_timer_cb(struct osmo_fsm_inst *fi)
 		 * could indicate a bad SCCP connection. We now inform the the
 		 * FSM that controls the BSSMAP reset about the event. Maybe
 		 * a BSSMAP reset is necessary. */
-		a_reset_conn_fail(conn->sccp.msc->a.reset_fsm);
+		a_reset_conn_fail(conn->sccp.msc);
 
 		/* Since we could not reach the MSC, we give up and terminate
 		 * the FSM instance now (N-DISCONNET.req is sent in
@@ -783,7 +783,7 @@ static int gscon_timer_cb(struct osmo_fsm_inst *fi)
 		 * disconnected. */
 		LOGPFSML(fi, LOGL_ERROR, "Long after a BSSMAP Clear Command, the conn is still not"
 			 " released. For sanity, discarding this conn now.\n");
-		a_reset_conn_fail(conn->sccp.msc->a.reset_fsm);
+		a_reset_conn_fail(conn->sccp.msc);
 		osmo_fsm_inst_term(fi, OSMO_FSM_TERM_ERROR, NULL);
 		break;
 	default:
diff --git a/src/osmo-bsc/gsm_08_08.c b/src/osmo-bsc/gsm_08_08.c
index d1a6a96ee..0d7cdf0d5 100644
--- a/src/osmo-bsc/gsm_08_08.c
+++ b/src/osmo-bsc/gsm_08_08.c
@@ -48,7 +48,7 @@ static bool msc_connected(struct gsm_subscriber_connection *conn)
 		return false;
 
 	/* Reset procedure not (yet) executed */
-	if (a_reset_conn_ready(conn->sccp.msc->a.reset_fsm) == false)
+	if (a_reset_conn_ready(conn->sccp.msc) == false)
 		return false;
 
 	return true;
diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c
index 95bad7b16..c067dd3a8 100644
--- a/src/osmo-bsc/osmo_bsc_bssap.c
+++ b/src/osmo-bsc/osmo_bsc_bssap.c
@@ -59,7 +59,7 @@ static int bssmap_handle_reset_ack(struct bsc_msc_data *msc,
 
 	/* Inform the FSM that controls the RESET/RESET-ACK procedure
 	 * that we have successfully received the reset-ack message */
-	a_reset_ack_confirm(msc->a.reset_fsm);
+	a_reset_ack_confirm(msc);
 
 	return 0;
 }
diff --git a/src/osmo-bsc/osmo_bsc_sigtran.c b/src/osmo-bsc/osmo_bsc_sigtran.c
index b97d51bce..7e5f5f68e 100644
--- a/src/osmo-bsc/osmo_bsc_sigtran.c
+++ b/src/osmo-bsc/osmo_bsc_sigtran.c
@@ -240,7 +240,7 @@ static int sccp_sap_up(struct osmo_prim_hdr *oph, void *_scu)
 		/* Incoming data is a sign of a vital connection */
 		conn = get_bsc_conn_by_conn_id(scu_prim->u.data.conn_id);
 		if (conn) {
-			a_reset_conn_success(conn->sccp.msc->a.reset_fsm);
+			a_reset_conn_success(conn->sccp.msc);
 			handle_data_from_msc(conn, oph->msg);
 		}
 		break;
@@ -284,7 +284,7 @@ enum bsc_con osmo_bsc_sigtran_new_conn(struct gsm_subscriber_connection *conn, s
 	LOGP(DMSC, LOGL_NOTICE, "Initializing resources for new SIGTRAN connection to MSC: %s...\n",
 	     osmo_sccp_addr_name(ss7, &msc->a.msc_addr));
 
-	if (a_reset_conn_ready(msc->a.reset_fsm) == false) {
+	if (a_reset_conn_ready(msc) == false) {
 		LOGP(DMSC, LOGL_ERROR, "MSC is not connected. Dropping.\n");
 		return BSC_CON_REJECT_NO_LINK;
 	}
@@ -314,7 +314,7 @@ int osmo_bsc_sigtran_open_conn(struct gsm_subscriber_connection *conn, struct ms
 
 	msc = conn->sccp.msc;
 
-	if (a_reset_conn_ready(msc->a.reset_fsm) == false) {
+	if (a_reset_conn_ready(msc) == false) {
 		LOGP(DMSC, LOGL_ERROR, "MSC is not connected. Dropping.\n");
 		return -EINVAL;
 	}
@@ -373,7 +373,7 @@ int osmo_bsc_sigtran_send(struct gsm_subscriber_connection *conn, struct msgb *m
 	} else
 		LOGP(DMSC, LOGL_ERROR, "Tx MSC (message too short)\n");
 
-	if (a_reset_conn_ready(msc->a.reset_fsm) == false) {
+	if (a_reset_conn_ready(msc) == false) {
 		LOGP(DMSC, LOGL_ERROR, "MSC is not connected. Dropping.\n");
 		msgb_free(msg);
 		return -EINVAL;
@@ -532,9 +532,7 @@ int osmo_bsc_sigtran_init(struct llist_head *mscs)
 			return -EINVAL;
 
 		/* Start MSC-Reset procedure */
-		msc->a.reset_fsm = a_reset_alloc(msc, msc_name, osmo_bsc_sigtran_reset_cb, msc);
-		if (!msc->a.reset_fsm)
-			return -EINVAL;
+		a_reset_alloc(msc, msc_name, osmo_bsc_sigtran_reset_cb);
 
 		/* If we have detected that the SS7 configuration of the MSC we have just initalized
 		 * was incomplete or completely missing, we can not tolerate another incomplete