[PATCH] osmo-hlr[master]: Update DB routines

Max gerrit-no-reply at lists.osmocom.org
Thu Feb 16 12:44:06 UTC 2017


Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/1840

to look at the new patch set (#2).

Update DB routines

* add SQL queries to change nam_ps value and function which uses them
* move common cleanup code into separate function
* add helper function for IMSI binding
* fix printf security warning
* use errno.h instead of numbers
* code cleanup

Change-Id: I24fb79e084b2dfa6a81b52f448b94a86e47014ef
---
M src/db.c
M src/db.h
M src/db_hlr.c
3 files changed, 75 insertions(+), 50 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/40/1840/2

diff --git a/src/db.c b/src/db.c
index 1385502..aa4726c 100644
--- a/src/db.c
+++ b/src/db.c
@@ -19,6 +19,7 @@
 
 #include <osmocom/core/utils.h>
 
+#include <stdbool.h>
 #include <sqlite3.h>
 
 #include "logging.h"
@@ -32,6 +33,8 @@
 	[AUC_UPD_SQN] = "UPDATE auc_3g SET sqn = ? WHERE subscriber_id = ?",
 	[UPD_PURGE_CS_BY_IMSI] = "UPDATE subscriber SET ms_purged_cs=1 WHERE imsi = ?",
 	[UPD_PURGE_PS_BY_IMSI] = "UPDATE subscriber SET ms_purged_ps=1 WHERE imsi = ?",
+	[SET_NAM_PS_BY_IMSI] = "UPDATE subscriber SET nam_ps=1 WHERE imsi = ?",
+	[UNSET_NAM_PS_BY_IMSI] = "UPDATE subscriber SET nam_ps=0 WHERE imsi = ?",
 };
 
 static void sql3_error_log_cb(void *arg, int err_code, const char *msg)
@@ -46,7 +49,7 @@
 		LOGP(DDB, LOGL_DEBUG, "Opened database\n");
 		break;
 	case 1:
-		LOGP(DDB, LOGL_DEBUG, stmt);
+		LOGP(DDB, LOGL_DEBUG, "%s\n", stmt);
 		break;
 	case 2:
 		LOGP(DDB, LOGL_DEBUG, "Closed database\n");
@@ -57,6 +60,36 @@
 	}
 }
 
+/* remove bindings and reset statement to be re-executed */
+bool db_remove_reset(sqlite3_stmt *stmt)
+{
+	int rc = sqlite3_clear_bindings(stmt);
+	if (rc != SQLITE_OK) {
+		LOGP(DDB, LOGL_ERROR, "Error clerearing bindings: %d\n", rc);
+		return false;
+	}
+
+	rc = sqlite3_reset(stmt);
+	if (rc != SQLITE_OK) {
+		LOGP(DDB, LOGL_ERROR, "Error in sqlite3_reset: %d\n", rc);
+		return false;
+	}
+	return true;
+}
+
+/* bind IMSI and do proper cleanup in case of failure */
+bool db_bind_imsi(sqlite3_stmt *stmt, const char *imsi)
+{
+	int rc = sqlite3_bind_text(stmt, 1, imsi, -1, SQLITE_STATIC);
+	if (rc != SQLITE_OK) {
+		LOGP(DDB, LOGL_ERROR, "Error binding IMSI %s: %d\n", imsi, rc);
+		db_remove_reset(stmt);
+		return false;
+	}
+
+	return true;
+}
+
 void db_close(struct db_context *dbc)
 {
 	unsigned int i;
diff --git a/src/db.h b/src/db.h
index d569fb0..b441d6a 100644
--- a/src/db.h
+++ b/src/db.h
@@ -9,8 +9,10 @@
 	UPD_SGSN_BY_ID	= 2,
 	AUC_BY_IMSI	= 3,
 	AUC_UPD_SQN	= 4,
-	UPD_PURGE_CS_BY_IMSI,
-	UPD_PURGE_PS_BY_IMSI,
+	UPD_PURGE_CS_BY_IMSI = 5,
+	UPD_PURGE_PS_BY_IMSI = 6,
+	SET_NAM_PS_BY_IMSI = 7,
+	UNSET_NAM_PS_BY_IMSI = 8,
 	_NUM_STMT
 };
 
@@ -20,6 +22,8 @@
 	sqlite3_stmt *stmt[_NUM_STMT];
 };
 
+bool db_remove_reset(sqlite3_stmt *stmt);
+bool db_bind_imsi(sqlite3_stmt *stmt, const char *imsi);
 void db_close(struct db_context *dbc);
 struct db_context *db_open(void *ctx, const char *fname);
 
@@ -68,7 +72,7 @@
 
 int db_subscr_get(struct db_context *dbc, const char *imsi,
 		  struct hlr_subscriber *subscr);
-
+int db_subscr_ps(struct db_context *dbc, const char *imsi, bool enable);
 int db_subscr_lu(struct db_context *dbc,
 		 const struct hlr_subscriber *subscr,
 		 const char *vlr_or_sgsn_number,
diff --git a/src/db_hlr.c b/src/db_hlr.c
index 1b95556..a2b708f 100644
--- a/src/db_hlr.c
+++ b/src/db_hlr.c
@@ -18,6 +18,7 @@
  */
 
 #include <string.h>
+#include <errno.h>
 
 #include <osmocom/core/utils.h>
 #include <osmocom/crypt/auth.h>
@@ -42,17 +43,14 @@
 	sqlite3_stmt *stmt = dbc->stmt[SEL_BY_IMSI];
 	int rc, ret = 0;
 
-	rc = sqlite3_bind_text(stmt, 1, imsi, -1, SQLITE_STATIC);
-	if (rc != SQLITE_OK) {
-		LOGHLR(imsi, LOGL_ERROR, "Error binding IMSI: %d\n", rc);
-		return -1;
-	}
+	if (!db_bind_imsi(stmt, imsi))
+		return -EINVAL;
 
 	/* execute the statement */
 	rc = sqlite3_step(stmt);
 	if (rc != SQLITE_ROW) {
 		LOGHLR(imsi, LOGL_ERROR, "Error executing SQL: %d\n", rc);
-		ret = -2;
+		ret = -ENOEXEC;
 		goto out;
 	}
 
@@ -73,17 +71,29 @@
 	subscr->ms_purged_ps = sqlite3_column_int(stmt, 12);
 
 out:
-	/* remove bindings and reset statement to be re-executed */
-	rc = sqlite3_clear_bindings(stmt);
-	if (rc != SQLITE_OK) {
-		LOGP(DAUC, LOGL_ERROR, "Error clerearing bindings: %d\n", rc);
-	}
-	rc = sqlite3_reset(stmt);
-	if (rc != SQLITE_OK) {
-		LOGP(DAUC, LOGL_ERROR, "Error in sqlite3_reset: %d\n", rc);
-	}
+	db_remove_reset(stmt);
 
 	return ret;
+}
+
+int db_subscr_ps(struct db_context *dbc, const char *imsi, bool enable)
+{
+	sqlite3_stmt *stmt =
+		dbc->stmt[enable ? SET_NAM_PS_BY_IMSI : UNSET_NAM_PS_BY_IMSI];
+	int rc = 0;
+
+	if (!db_bind_imsi(stmt, imsi))
+		return -EINVAL;
+
+	/* execute the statement */
+	rc = sqlite3_step(stmt);
+	if (rc != SQLITE_DONE) {
+		LOGHLR(imsi, LOGL_ERROR, "Error executing SQL: %d\n", rc);
+		rc = -ENOEXEC;
+	}
+
+	db_remove_reset(stmt);
+	return rc;
 }
 
 int db_subscr_lu(struct db_context *dbc,
@@ -105,13 +115,13 @@
 	rc = sqlite3_bind_int64(stmt, 1, subscr->id);
 	if (rc != SQLITE_OK) {
 		LOGP(DAUC, LOGL_ERROR, "Error binding ID: %d\n", rc);
-		return -1;
+		return -EINVAL;
 	}
 
 	rc = sqlite3_bind_text(stmt, 2, txt, -1, SQLITE_STATIC);
 	if (rc != SQLITE_OK) {
 		LOGP(DAUC, LOGL_ERROR, "Error binding VLR/SGSN Number: %d\n", rc);
-		ret = -2;
+		ret = -ECHRNG;
 		goto out;
 	}
 
@@ -119,19 +129,10 @@
 	rc = sqlite3_step(stmt);
 	if (rc != SQLITE_DONE) {
 		LOGP(DAUC, LOGL_ERROR, "Error updating SQN: %d\n", rc);
-		ret = -3;
-		goto out;
+		ret = -ENOEXEC;
 	}
 out:
-	/* remove bindings and reset statement to be re-executed */
-	rc = sqlite3_clear_bindings(stmt);
-	if (rc != SQLITE_OK) {
-		LOGP(DAUC, LOGL_ERROR, "Error clerearing bindings: %d\n", rc);
-	}
-	rc = sqlite3_reset(stmt);
-	if (rc != SQLITE_OK) {
-		LOGP(DAUC, LOGL_ERROR, "Error in sqlite3_reset: %d\n", rc);
-	}
+	db_remove_reset(stmt);
 
 	return ret;
 }
@@ -146,31 +147,18 @@
 	else
 		stmt = dbc->stmt[UPD_PURGE_CS_BY_IMSI];
 
-	rc = sqlite3_bind_text(stmt, 1, imsi, -1, SQLITE_STATIC);
-	if (rc != SQLITE_OK) {
-		LOGP(DAUC, LOGL_ERROR, "Error binding IMSI %s: %d\n", imsi, rc);
-		ret = -1;
-		goto out;
-	}
+	if (!db_bind_imsi(stmt, imsi))
+		return -EINVAL;
 
 	/* execute the statement */
 	rc = sqlite3_step(stmt);
 	if (rc != SQLITE_DONE) {
 		LOGP(DAUC, LOGL_ERROR, "Error setting Purged: %d\n", rc);
-		ret = -2;
-		goto out;
+		ret = -ENOEXEC;
 	}
 	/* FIXME: return 0 in case IMSI not known */
-out:
-	/* remove bindings and reset statement to be re-executed */
-	rc = sqlite3_clear_bindings(stmt);
-	if (rc != SQLITE_OK) {
-		LOGP(DAUC, LOGL_ERROR, "Error clearing bindings: %d\n", rc);
-	}
-	rc = sqlite3_reset(stmt);
-	if (rc != SQLITE_OK) {
-		LOGP(DAUC, LOGL_ERROR, "Error in sqlite3_reset: %d\n", rc);
-	}
+
+	db_remove_reset(stmt);
 
 	return ret;
 }

-- 
To view, visit https://gerrit.osmocom.org/1840
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I24fb79e084b2dfa6a81b52f448b94a86e47014ef
Gerrit-PatchSet: 2
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder


More information about the gerrit-log mailing list