This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Neels Hofmeyr gerrit-no-reply at lists.osmocom.orgHello Harald Welte, Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/4182
to look at the new patch set (#3).
refactor db_subscr_ps() to db_subscr_nam()
Allow to set nam_ps and nam_cs from this same function, by adding the is_ps
arg.
Combine both NAM_PS stmts to DB_STMT_UPD_NAM_PS_BY_IMSI, add another such stmt
for CS. Use named parameters instead of parameter indexes.
Improve error return values as well as error logging to clearly indicate
whether the operation could not find the requested IMSI, or other errors
occured.
Adjust the single caller.
This prepares for upcoming VTY and possibly CTRL commands, and the error
handling introduced here has been or will be adopted by other functions in
previous or subsequent patches.
Change-Id: I6e70e15228f5bb10bee6758ae5dc9687d65839bd
---
M src/ctrl.c
M src/db.c
M src/db.h
M src/db_hlr.c
M tests/db/db_test.c
M tests/db/db_test.err
6 files changed, 230 insertions(+), 20 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/82/4182/3
diff --git a/src/ctrl.c b/src/ctrl.c
index b034cd7..8682e14 100644
--- a/src/ctrl.c
+++ b/src/ctrl.c
@@ -43,7 +43,7 @@
return CTRL_CMD_ERROR;
}
- if (db_subscr_ps(ctx->dbc, cmd->value, enable) < 0) {
+ if (db_subscr_nam(ctx->dbc, cmd->value, enable, true) < 0) {
cmd->reply = "Error updating DB";
return CTRL_CMD_ERROR;
}
diff --git a/src/db.c b/src/db.c
index 5a38d55..8286ba8 100644
--- a/src/db.c
+++ b/src/db.c
@@ -56,8 +56,8 @@
[DB_STMT_AUC_UPD_SQN] = "UPDATE auc_3g SET sqn = ? WHERE subscriber_id = ?",
[DB_STMT_UPD_PURGE_CS_BY_IMSI] = "UPDATE subscriber SET ms_purged_cs=1 WHERE imsi = ?",
[DB_STMT_UPD_PURGE_PS_BY_IMSI] = "UPDATE subscriber SET ms_purged_ps=1 WHERE imsi = ?",
- [DB_STMT_SET_NAM_PS_BY_IMSI] = "UPDATE subscriber SET nam_ps=1 WHERE imsi = ?",
- [DB_STMT_UNSET_NAM_PS_BY_IMSI] = "UPDATE subscriber SET nam_ps=0 WHERE imsi = ?",
+ [DB_STMT_UPD_NAM_CS_BY_IMSI] = "UPDATE subscriber SET nam_cs = $val WHERE imsi = $imsi",
+ [DB_STMT_UPD_NAM_PS_BY_IMSI] = "UPDATE subscriber SET nam_ps = $val WHERE imsi = $imsi",
[DB_STMT_SUBSCR_CREATE] = "INSERT INTO subscriber (imsi) VALUES ($imsi)",
[DB_STMT_DEL_BY_ID] = "DELETE FROM subscriber WHERE id = $subscriber_id",
[DB_STMT_SET_MSISDN_BY_IMSI] = "UPDATE subscriber SET msisdn = $msisdn WHERE imsi = $imsi",
diff --git a/src/db.h b/src/db.h
index 5ec7b72..5e234ff 100644
--- a/src/db.h
+++ b/src/db.h
@@ -13,8 +13,8 @@
DB_STMT_AUC_UPD_SQN,
DB_STMT_UPD_PURGE_CS_BY_IMSI,
DB_STMT_UPD_PURGE_PS_BY_IMSI,
- DB_STMT_SET_NAM_PS_BY_IMSI,
- DB_STMT_UNSET_NAM_PS_BY_IMSI,
+ DB_STMT_UPD_NAM_PS_BY_IMSI,
+ DB_STMT_UPD_NAM_CS_BY_IMSI,
DB_STMT_SUBSCR_CREATE,
DB_STMT_DEL_BY_ID,
DB_STMT_SET_MSISDN_BY_IMSI,
@@ -90,7 +90,7 @@
struct hlr_subscriber *subscr);
int db_subscr_get_by_id(struct db_context *dbc, int64_t id,
struct hlr_subscriber *subscr);
-int db_subscr_ps(struct db_context *dbc, const char *imsi, bool enable);
+int db_subscr_nam(struct db_context *dbc, const char *imsi, bool nam_val, bool is_ps);
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 8df4bcb..9bdb372 100644
--- a/src/db_hlr.c
+++ b/src/db_hlr.c
@@ -259,31 +259,58 @@
return rc;
}
-int db_subscr_ps(struct db_context *dbc, const char *imsi, bool enable)
+/* Enable or disable PS or CS for a subscriber.
+ * For the subscriber with the given imsi, set nam_ps (when is_ps == true) or
+ * nam_cs (when is_ps == false) to nam_val in the database.
+ * Returns 0 on success, -ENOENT when the given IMSI does not exist, -EINVAL if
+ * the SQL statement could not be composed, -ENOEXEC if running the SQL
+ * statement failed, -EIO if the amount of rows modified is unexpected.
+ */
+int db_subscr_nam(struct db_context *dbc, const char *imsi, bool nam_val, bool is_ps)
{
- sqlite3_stmt *stmt =
- dbc->stmt[enable ? DB_STMT_SET_NAM_PS_BY_IMSI : DB_STMT_UNSET_NAM_PS_BY_IMSI];
+ sqlite3_stmt *stmt;
int rc;
+ int ret = 0;
- if (!db_bind_text(stmt, NULL, imsi))
- return -EINVAL;
+ stmt = dbc->stmt[is_ps ? DB_STMT_UPD_NAM_PS_BY_IMSI
+ : DB_STMT_UPD_NAM_CS_BY_IMSI];
- rc = sqlite3_step(stmt); /* execute the statement */
+ if (!db_bind_text(stmt, "$imsi", imsi))
+ return -EIO;
+ if (!db_bind_int(stmt, "$val", nam_val ? 1 : 0))
+ return -EIO;
+
+ /* execute the statement */
+ rc = sqlite3_step(stmt);
if (rc != SQLITE_DONE) {
- LOGHLR(imsi, LOGL_ERROR, "Error executing SQL: %d\n", rc);
- db_remove_reset(stmt);
- return -ENOEXEC;
+ LOGHLR(imsi, LOGL_ERROR, "%s %s: SQL error: %s\n",
+ nam_val ? "enable" : "disable",
+ is_ps ? "PS" : "CS",
+ sqlite3_errmsg(dbc->db));
+ ret = -EIO;
+ goto out;
}
- rc = sqlite3_changes(dbc->db); /* verify execution result */
- if (rc != 1) {
- LOGHLR(imsi, LOGL_ERROR, "SQL modified %d rows (expected 1)\n",
+ /* verify execution result */
+ rc = sqlite3_changes(dbc->db);
+ if (!rc) {
+ LOGP(DAUC, LOGL_ERROR, "Cannot %s %s: no such subscriber: IMSI='%s'\n",
+ nam_val ? "enable" : "disable",
+ is_ps ? "PS" : "CS",
+ imsi);
+ ret = -ENOENT;
+ goto out;
+ } else if (rc != 1) {
+ LOGHLR(imsi, LOGL_ERROR, "%s %s: SQL modified %d rows (expected 1)\n",
+ nam_val ? "enable" : "disable",
+ is_ps ? "PS" : "CS",
rc);
- rc = -EINVAL;
+ ret = -EIO;
}
+out:
db_remove_reset(stmt);
- return rc;
+ return ret;
}
int db_subscr_lu(struct db_context *dbc,
diff --git a/tests/db/db_test.c b/tests/db/db_test.c
index 250b363..30de51d 100644
--- a/tests/db/db_test.c
+++ b/tests/db/db_test.c
@@ -244,6 +244,45 @@
ASSERT_RC(db_subscr_update_msisdn_by_imsi(dbc, "foobar", "99"), -ENOENT);
ASSERT_SEL(msisdn, "99", -ENOENT);
+ comment("Set / unset nam_cs and nam_ps");
+
+ /* nam_val, is_ps */
+ ASSERT_RC(db_subscr_nam(dbc, imsi0, false, true), 0);
+ ASSERT_SEL(imsi, imsi0, 0);
+ ASSERT_RC(db_subscr_nam(dbc, imsi0, false, false), 0);
+ ASSERT_SEL(imsi, imsi0, 0);
+ ASSERT_RC(db_subscr_nam(dbc, imsi0, true, false), 0);
+ ASSERT_SEL(imsi, imsi0, 0);
+ ASSERT_RC(db_subscr_nam(dbc, imsi0, true, true), 0);
+ ASSERT_SEL(imsi, imsi0, 0);
+
+ comment("Set / unset nam_cs and nam_ps *again*");
+ ASSERT_RC(db_subscr_nam(dbc, imsi0, false, true), 0);
+ ASSERT_SEL(imsi, imsi0, 0);
+ ASSERT_RC(db_subscr_nam(dbc, imsi0, false, true), 0);
+ ASSERT_SEL(imsi, imsi0, 0);
+ ASSERT_RC(db_subscr_nam(dbc, imsi0, false, false), 0);
+ ASSERT_SEL(imsi, imsi0, 0);
+ ASSERT_RC(db_subscr_nam(dbc, imsi0, false, false), 0);
+ ASSERT_SEL(imsi, imsi0, 0);
+ ASSERT_RC(db_subscr_nam(dbc, imsi0, true, true), 0);
+ ASSERT_SEL(imsi, imsi0, 0);
+ ASSERT_RC(db_subscr_nam(dbc, imsi0, true, true), 0);
+ ASSERT_SEL(imsi, imsi0, 0);
+ ASSERT_RC(db_subscr_nam(dbc, imsi0, true, false), 0);
+ ASSERT_SEL(imsi, imsi0, 0);
+ ASSERT_RC(db_subscr_nam(dbc, imsi0, true, false), 0);
+ ASSERT_SEL(imsi, imsi0, 0);
+
+ comment("Set nam_cs and nam_ps on non-existent / invalid IMSI");
+
+ ASSERT_RC(db_subscr_nam(dbc, unknown_imsi, false, true), -ENOENT);
+ ASSERT_RC(db_subscr_nam(dbc, unknown_imsi, false, false), -ENOENT);
+ ASSERT_SEL(imsi, unknown_imsi, -ENOENT);
+
+ ASSERT_RC(db_subscr_nam(dbc, "foobar", false, true), -ENOENT);
+ ASSERT_RC(db_subscr_nam(dbc, "foobar", false, false), -ENOENT);
+
comment("Delete non-existent / invalid IDs");
ASSERT_RC(db_subscr_delete_by_id(dbc, 999), -ENOENT);
diff --git a/tests/db/db_test.err b/tests/db/db_test.err
index 17323b5..f7c03d9 100644
--- a/tests/db/db_test.err
+++ b/tests/db/db_test.err
@@ -232,6 +232,150 @@
DAUC Cannot read subscriber from db: MSISDN='99': No such subscriber
+--- Set / unset nam_cs and nam_ps
+
+db_subscr_nam(dbc, imsi0, false, true) --> 0
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+ .id = 1,
+ .imsi = '123456789000000',
+ .msisdn = '543210123456789',
+ .nam_ps = false,
+}
+
+db_subscr_nam(dbc, imsi0, false, false) --> 0
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+ .id = 1,
+ .imsi = '123456789000000',
+ .msisdn = '543210123456789',
+ .nam_cs = false,
+ .nam_ps = false,
+}
+
+db_subscr_nam(dbc, imsi0, true, false) --> 0
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+ .id = 1,
+ .imsi = '123456789000000',
+ .msisdn = '543210123456789',
+ .nam_ps = false,
+}
+
+db_subscr_nam(dbc, imsi0, true, true) --> 0
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+ .id = 1,
+ .imsi = '123456789000000',
+ .msisdn = '543210123456789',
+}
+
+
+--- Set / unset nam_cs and nam_ps *again*
+
+db_subscr_nam(dbc, imsi0, false, true) --> 0
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+ .id = 1,
+ .imsi = '123456789000000',
+ .msisdn = '543210123456789',
+ .nam_ps = false,
+}
+
+db_subscr_nam(dbc, imsi0, false, true) --> 0
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+ .id = 1,
+ .imsi = '123456789000000',
+ .msisdn = '543210123456789',
+ .nam_ps = false,
+}
+
+db_subscr_nam(dbc, imsi0, false, false) --> 0
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+ .id = 1,
+ .imsi = '123456789000000',
+ .msisdn = '543210123456789',
+ .nam_cs = false,
+ .nam_ps = false,
+}
+
+db_subscr_nam(dbc, imsi0, false, false) --> 0
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+ .id = 1,
+ .imsi = '123456789000000',
+ .msisdn = '543210123456789',
+ .nam_cs = false,
+ .nam_ps = false,
+}
+
+db_subscr_nam(dbc, imsi0, true, true) --> 0
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+ .id = 1,
+ .imsi = '123456789000000',
+ .msisdn = '543210123456789',
+ .nam_cs = false,
+}
+
+db_subscr_nam(dbc, imsi0, true, true) --> 0
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+ .id = 1,
+ .imsi = '123456789000000',
+ .msisdn = '543210123456789',
+ .nam_cs = false,
+}
+
+db_subscr_nam(dbc, imsi0, true, false) --> 0
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+ .id = 1,
+ .imsi = '123456789000000',
+ .msisdn = '543210123456789',
+}
+
+db_subscr_nam(dbc, imsi0, true, false) --> 0
+
+db_subscr_get_by_imsi(dbc, imsi0, &g_subscr) --> 0
+struct hlr_subscriber {
+ .id = 1,
+ .imsi = '123456789000000',
+ .msisdn = '543210123456789',
+}
+
+
+--- Set nam_cs and nam_ps on non-existent / invalid IMSI
+
+db_subscr_nam(dbc, unknown_imsi, false, true) --> -ENOENT
+DAUC Cannot disable PS: no such subscriber: IMSI='999999999'
+
+db_subscr_nam(dbc, unknown_imsi, false, false) --> -ENOENT
+DAUC Cannot disable CS: no such subscriber: IMSI='999999999'
+
+db_subscr_get_by_imsi(dbc, unknown_imsi, &g_subscr) --> -ENOENT
+DAUC Cannot read subscriber from db: IMSI='999999999': No such subscriber
+
+db_subscr_nam(dbc, "foobar", false, true) --> -ENOENT
+DAUC Cannot disable PS: no such subscriber: IMSI='foobar'
+
+db_subscr_nam(dbc, "foobar", false, false) --> -ENOENT
+DAUC Cannot disable CS: no such subscriber: IMSI='foobar'
+
+
--- Delete non-existent / invalid IDs
db_subscr_delete_by_id(dbc, 999) --> -ENOENT
--
To view, visit https://gerrit.osmocom.org/4182
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6e70e15228f5bb10bee6758ae5dc9687d65839bd
Gerrit-PatchSet: 3
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder