Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/7629
to look at the new patch set (#2).
add osmo_fsm_inst_update_id_f()
In the osmo-msc, I would like to set the subscr conn FSM identifier by a string
format, to include the type of Complete Layer 3 that is taking place. I could
each time talloc a string and free it again. This API is more convenient.
>From osmo_fsm_inst_update_id(), call osmo_fsm_inst_update_id_f() with "%s" (or
pass NULL).
Put the name updating into separate static update_name() function to clarify.
Adjust the error message for erratic ID: don't say "allocate", it might be from
an update. Adjust test expectation.
Change-Id: I76743a7642f2449fd33350691ac8ebbf4400371d
---
M include/osmocom/core/fsm.h
M src/fsm.c
M tests/fsm/fsm_test.c
M tests/fsm/fsm_test.err
4 files changed, 97 insertions(+), 22 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/29/7629/2
diff --git a/include/osmocom/core/fsm.h b/include/osmocom/core/fsm.h
index 2c2a996..174396a 100644
--- a/include/osmocom/core/fsm.h
+++ b/include/osmocom/core/fsm.h
@@ -159,6 +159,7 @@
void osmo_fsm_inst_free(struct osmo_fsm_inst *fi);
int osmo_fsm_inst_update_id(struct osmo_fsm_inst *fi, const char *id);
+int osmo_fsm_inst_update_id_f(struct osmo_fsm_inst *fi, const char *fmt, ...);
const char *osmo_fsm_event_name(struct osmo_fsm *fsm, uint32_t event);
const char *osmo_fsm_inst_name(struct osmo_fsm_inst *fi);
diff --git a/src/fsm.c b/src/fsm.c
index 0fdc564..ae2fdc2 100644
--- a/src/fsm.c
+++ b/src/fsm.c
@@ -211,38 +211,61 @@
*/
int osmo_fsm_inst_update_id(struct osmo_fsm_inst *fi, const char *id)
{
- if (id) {
+ if (!id)
+ return osmo_fsm_inst_update_id_f(fi, NULL);
+ else
+ return osmo_fsm_inst_update_id_f(fi, "%s", id);
+}
+
+static void update_name(struct osmo_fsm_inst *fi)
+{
+ if (fi->name)
+ talloc_free((char*)fi->name);
+
+ if (!fsm_log_addr) {
+ if (fi->id)
+ fi->name = talloc_asprintf(fi, "%s(%s)", fi->fsm->name,
fi->id);
+ else
+ fi->name = talloc_asprintf(fi, "%s", fi->fsm->name);
+ } else {
+ if (fi->id)
+ fi->name = talloc_asprintf(fi, "%s(%s)[%p]",
fi->fsm->name, fi->id, fi);
+ else
+ fi->name = talloc_asprintf(fi, "%s[%p]", fi->fsm->name,
fi);
+ }
+}
+
+/*! Change id of the FSM instance using a string format.
+ * \param[in] fi FSM instance.
+ * \param[in] fmt format string to compose new ID.
+ * \param[in] ... variable argument list for format string.
+ * \returns 0 if the ID was updated, otherwise -EINVAL.
+ */
+int osmo_fsm_inst_update_id_f(struct osmo_fsm_inst *fi, const char *fmt, ...)
+{
+ char *id = NULL;
+
+ if (fmt) {
+ va_list ap;
+
+ va_start(ap, fmt);
+ id = talloc_vasprintf(fi, fmt, ap);
+ va_end(ap);
+
if (!osmo_identifier_valid(id)) {
LOGP(DLGLOBAL, LOGL_ERROR,
"Attempting to set illegal id for FSM instance of
type '%s': %s\n",
fi->fsm->name, osmo_quote_str(id, -1));
+ talloc_free(id);
return -EINVAL;
}
- osmo_talloc_replace_string(fi, (char **)&fi->id, id);
-
- if (fi->name)
- talloc_free((void*)fi->name);
-
- if (!fsm_log_addr) {
- fi->name = talloc_asprintf(fi, "%s(%s)", fi->fsm->name,
id);
- } else {
- fi->name = talloc_asprintf(fi, "%s(%s)[%p]",
fi->fsm->name, id, fi);
- }
-
- return 0;
}
if (fi->id)
- talloc_free((void*)fi->id);
- fi->id = NULL;
- if (fi->name)
- talloc_free((void*)fi->name);
+ talloc_free((char*)fi->id);
+ fi->id = id;
- if (!fsm_log_addr) {
- fi->name = talloc_asprintf(fi, "%s", fi->fsm->name);
- } else {
- fi->name = talloc_asprintf(fi, "%s[%p]", fi->fsm->name, fi);
- }
+ update_name(fi);
return 0;
}
diff --git a/tests/fsm/fsm_test.c b/tests/fsm/fsm_test.c
index 859b78d..e34164c 100644
--- a/tests/fsm/fsm_test.c
+++ b/tests/fsm/fsm_test.c
@@ -237,6 +237,30 @@
test_id("invalid.id", -EINVAL, "(arbitrary_id)");
+ fprintf(stderr, "--- id format tests...\n");
+/* Update the id, assert the proper rc, and expect a resulting fsm inst name +
lookup */
+#define test_id_f(expect_rc, expect_name_suffix, new_id_fmt, args...) do { \
+ int rc; \
+ fprintf(stderr, "osmo_fsm_inst_update_id_f(%s, " #args ")\n", \
+ osmo_quote_str(new_id_fmt, -1)); \
+ rc = osmo_fsm_inst_update_id_f(fi, new_id_fmt, ## args); \
+ fprintf(stderr, " rc == %d", rc); \
+ if (rc == (expect_rc)) \
+ fprintf(stderr, ", ok\n"); \
+ else { \
+ fprintf(stderr, ", ERROR: expected rc == %d\n",
expect_rc); \
+ OSMO_ASSERT(rc == expect_rc); \
+ } \
+ assert_name("Test_FSM" expect_name_suffix); \
+ }while (0)
+
+ test_id_f(-EINVAL, "(arbitrary_id)", "format%cid", '.');
+ test_id_f(-EINVAL, "(arbitrary_id)", "%s", "");
+ test_id_f(0, "(format23id42)", "format%xid%d", 0x23, 42);
+ test_id_f(0, "", NULL);
+ test_id_f(0, "", NULL);
+ test_id_f(0, "(arbitrary_id)", "%s%c%s", "arbitrary", '_', "id");
+
fprintf(stderr, "\n--- %s() done\n\n", __func__);
osmo_fsm_inst_term(fi, OSMO_FSM_TERM_REQUEST, NULL);
diff --git a/tests/fsm/fsm_test.err b/tests/fsm/fsm_test.err
index 3e01d48..3237def 100644
--- a/tests/fsm/fsm_test.err
+++ b/tests/fsm/fsm_test.err
@@ -47,6 +47,33 @@
[0;m rc == -22, ok
osmo_fsm_inst_name() == "Test_FSM(arbitrary_id)"
osmo_fsm_inst_find_by_name("Test_FSM(arbitrary_id)") == fi
+--- id format tests...
+osmo_fsm_inst_update_id_f("format%cid", '.')
+Attempting to set illegal id for FSM instance of type 'Test_FSM': "format.id"
+[0;m rc == -22, ok
+ osmo_fsm_inst_name() == "Test_FSM(arbitrary_id)"
+ osmo_fsm_inst_find_by_name("Test_FSM(arbitrary_id)") == fi
+osmo_fsm_inst_update_id_f("%s", "")
+Attempting to set illegal id for FSM instance of type 'Test_FSM': ""
+[0;m rc == -22, ok
+ osmo_fsm_inst_name() == "Test_FSM(arbitrary_id)"
+ osmo_fsm_inst_find_by_name("Test_FSM(arbitrary_id)") == fi
+osmo_fsm_inst_update_id_f("format%xid%d", 0x23, 42)
+ rc == 0, ok
+ osmo_fsm_inst_name() == "Test_FSM(format23id42)"
+ osmo_fsm_inst_find_by_name("Test_FSM(format23id42)") == fi
+osmo_fsm_inst_update_id_f(NULL, )
+ rc == 0, ok
+ osmo_fsm_inst_name() == "Test_FSM"
+ osmo_fsm_inst_find_by_name("Test_FSM") == fi
+osmo_fsm_inst_update_id_f(NULL, )
+ rc == 0, ok
+ osmo_fsm_inst_name() == "Test_FSM"
+ osmo_fsm_inst_find_by_name("Test_FSM") == fi
+osmo_fsm_inst_update_id_f("%s%c%s", "arbitrary", '_', "id")
+ rc == 0, ok
+ osmo_fsm_inst_name() == "Test_FSM(arbitrary_id)"
+ osmo_fsm_inst_find_by_name("Test_FSM(arbitrary_id)") == fi
--- test_id_api() done
--
To view, visit https://gerrit.osmocom.org/7629
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I76743a7642f2449fd33350691ac8ebbf4400371d
Gerrit-PatchSet: 2
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
