gprs: Do not put the subscr in gprs_subscr_delete

Currently gprs_subscr_delete implicitely calls subscr_put, which
makes the code more complex than necessary (additional subscr_get) in
a few places. It also makes it more difficult to see, whether get/put
are balanced within a function. In addition, the functions are not
named consistently (gprs_subscr_delete vs.
gprs_subscr_put_and_cancel).

This commit changes the semantics of gprs_subscr_delete and
indirectly of gprs_subscr_put_and_cancel to not call subscr_put on
their argument, but to leave that for the caller to do it
explicitely.

It renames gprs_subscr_put_and_cancel to gprs_subscr_cancel to
reflect that change in the name, too.

Sponsored-by: On-Waves ehf

5 files changed, 10 insertions, 6 deletions

diff --git a/openbsc/include/openbsc/gprs_sgsn.h b/openbsc/include/openbsc/gprs_sgsn.h
index 79df21be0..e58b23b59 100644
--- a/openbsc/include/openbsc/gprs_sgsn.h
+++ b/openbsc/include/openbsc/gprs_sgsn.h
@@ -329,7 +329,7 @@ void gprs_subscr_delete(struct gsm_subscriber *subscr);
 struct gsm_subscriber *gprs_subscr_get_or_create(const char *imsi);
 struct gsm_subscriber *gprs_subscr_get_or_create_by_mmctx( struct sgsn_mm_ctx *mmctx);
 struct gsm_subscriber *gprs_subscr_get_by_imsi(const char *imsi);
-void gprs_subscr_put_and_cancel(struct gsm_subscriber *subscr);
+void gprs_subscr_cancel(struct gsm_subscriber *subscr);
 void gprs_subscr_update(struct gsm_subscriber *subscr);
 void gprs_subscr_update_auth_info(struct gsm_subscriber *subscr);
 int gprs_subscr_rx_gsup_message(struct msgb *msg);
diff --git a/openbsc/src/gprs/gprs_sgsn.c b/openbsc/src/gprs/gprs_sgsn.c
index acf37a28b..fdf4840b1 100644
--- a/openbsc/src/gprs/gprs_sgsn.c
+++ b/openbsc/src/gprs/gprs_sgsn.c
@@ -200,6 +200,7 @@ void sgsn_mm_ctx_free(struct sgsn_mm_ctx *mm)
 		mm->subscr = NULL;
 		subscr->sgsn_data->mm = NULL;
 		gprs_subscr_delete(subscr);
+		subscr_put(subscr);
 	}
 
 	/* Free all PDP contexts */
diff --git a/openbsc/src/gprs/gprs_subscriber.c b/openbsc/src/gprs/gprs_subscriber.c
index d6d874b0b..b7e6af415 100644
--- a/openbsc/src/gprs/gprs_subscriber.c
+++ b/openbsc/src/gprs/gprs_subscriber.c
@@ -141,10 +141,9 @@ void gprs_subscr_delete(struct gsm_subscriber *subscr)
 	}
 
 	subscr->keep_in_ram = 0;
-	subscr_put(subscr);
 }
 
-void gprs_subscr_put_and_cancel(struct gsm_subscriber *subscr)
+void gprs_subscr_cancel(struct gsm_subscriber *subscr)
 {
 	subscr->authorized = 0;
 	subscr->flags |= GPRS_SUBSCRIBER_CANCELLED;
@@ -380,7 +379,7 @@ static int gprs_subscr_handle_gsup_purge_res(struct gsm_subscriber *subscr,
 
 	/* Force silent cancellation */
 	subscr->sgsn_data->error_cause = SGSN_ERROR_CAUSE_NONE;
-	gprs_subscr_put_and_cancel(subscr_get(subscr));
+	gprs_subscr_cancel(subscr);
 
 	return 0;
 }
@@ -429,7 +428,7 @@ static int gprs_subscr_handle_loc_cancel_req(struct gsm_subscriber *subscr,
 	gprs_subscr_tx_gsup_message(subscr, &gsup_reply);
 
 	subscr->sgsn_data->error_cause = SGSN_ERROR_CAUSE_NONE;
-	gprs_subscr_put_and_cancel(subscr_get(subscr));
+	gprs_subscr_cancel(subscr);
 
 	return 0;
 }
diff --git a/openbsc/src/gprs/sgsn_vty.c b/openbsc/src/gprs/sgsn_vty.c
index b4e1150e6..fb6cb7857 100644
--- a/openbsc/src/gprs/sgsn_vty.c
+++ b/openbsc/src/gprs/sgsn_vty.c
@@ -560,7 +560,8 @@ DEFUN(update_subscr_cancel, update_subscr_cancel_cmd,
 		return CMD_WARNING;
 	}
 
-	gprs_subscr_put_and_cancel(subscr);
+	gprs_subscr_cancel(subscr);
+	subscr_put(subscr);
 
 	return CMD_SUCCESS;
 }
diff --git a/openbsc/tests/sgsn/sgsn_test.c b/openbsc/tests/sgsn/sgsn_test.c
index 34ad6473f..57577a440 100644
--- a/openbsc/tests/sgsn/sgsn_test.c
+++ b/openbsc/tests/sgsn/sgsn_test.c
@@ -270,6 +270,7 @@ static void test_subscriber(void)
 
 	/* There is no subscriber cache. Verify it */
 	gprs_subscr_delete(s1);
+	subscr_put(s1);
 	s1 = NULL;
 	sfound = gprs_subscr_get_by_imsi(imsi1);
 	OSMO_ASSERT(sfound == NULL);
@@ -279,6 +280,7 @@ static void test_subscriber(void)
 
 	/* Free entry 2 (GSM_SUBSCRIBER_FIRST_CONTACT is set) */
 	gprs_subscr_delete(s2);
+	subscr_put(s2);
 	s2 = NULL;
 	OSMO_ASSERT(gprs_subscr_get_by_imsi(imsi1) == NULL);
 	OSMO_ASSERT(gprs_subscr_get_by_imsi(imsi2) == NULL);
@@ -286,6 +288,7 @@ static void test_subscriber(void)
 
 	/* Try to delete entry 3 */
 	gprs_subscr_delete(s3);
+	subscr_put(s3);
 	s3 = NULL;
 	OSMO_ASSERT(gprs_subscr_get_by_imsi(imsi3) == NULL);