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.orgNeels Hofmeyr has submitted this change and it was merged. Change subject: fsm_test: more thoroughly test FSM inst ids and names ...................................................................... fsm_test: more thoroughly test FSM inst ids and names Place id and name testing in its separate section, test_id_api(). Add a test that actually allocates an FSM instance with a NULL id, which is allowed, but uncovers a bug of an unset FSM instance name. osmo_fsm_inst_name() falls back to the fsm struct's name on NULL, but osmo_fsm_inst_find_by_name() fails to match if the instance's name is NULL (and until recently even crashed). Show this in fsm_test.c with loud comments. Add test to clear the id by passing NULL. Add test for setting an empty id. Add test for setting an invalid identifier (osmo_identifier_valid() == false). Change-Id: I646ed918576ce196c395dc5f42a1507c52ace2c5 --- M tests/fsm/fsm_test.c M tests/fsm/fsm_test.err 2 files changed, 138 insertions(+), 7 deletions(-) Approvals: Harald Welte: Looks good to me, approved diff --git a/tests/fsm/fsm_test.c b/tests/fsm/fsm_test.c index ef7bfe3..0f29a2e 100644 --- a/tests/fsm/fsm_test.c +++ b/tests/fsm/fsm_test.c @@ -3,6 +3,7 @@ #include <stdarg.h> #include <unistd.h> #include <string.h> +#include <errno.h> #include <osmocom/core/utils.h> #include <osmocom/core/select.h> @@ -16,6 +17,12 @@ static void *g_ctx; +static int safe_strcmp(const char *a, const char *b) +{ + if (!a || !b) + return a == b ? 0 : 1; + return strcmp(a, b); +} enum test_fsm_states { ST_NULL = 0, @@ -118,7 +125,7 @@ struct ctrl_cmd *cmd; cmd = exec_ctrl_cmd(cmdstr); - if (strcmp(cmd->reply, expres)) { + if (safe_strcmp(cmd->reply, expres)) { fprintf(stderr, "Reply '%s' doesn't match expected '%s'\n", cmd->reply, expres); OSMO_ASSERT(0); } @@ -165,6 +172,89 @@ return fi; } +static void test_id_api() +{ + struct osmo_fsm_inst *fi; + + fprintf(stderr, "\n--- %s()\n", __func__); + +/* Assert the instance has this name and can be looked up by it */ +#define assert_name(expected_name) \ +do { \ + const char *name = osmo_fsm_inst_name(fi); \ + fprintf(stderr, " osmo_fsm_inst_name() == %s\n", osmo_quote_str(name, -1)); \ + if (safe_strcmp(name, expected_name)) { \ + fprintf(stderr, " ERROR: expected %s\n", osmo_quote_str(expected_name, -1)); \ + OSMO_ASSERT(false); \ + } \ + OSMO_ASSERT(osmo_fsm_inst_find_by_name(&fsm, expected_name) == fi); \ + fprintf(stderr, " osmo_fsm_inst_find_by_name(%s) == fi\n", osmo_quote_str(expected_name, -1)); \ +} while(0) + +/* Assert the instance can be looked up by this id string */ +#define assert_id(expected_id) \ +do { \ + OSMO_ASSERT(osmo_fsm_inst_find_by_id(&fsm, expected_id) == fi); \ + fprintf(stderr, " osmo_fsm_inst_find_by_id(%s) == fi\n", osmo_quote_str(expected_id, -1)); \ +} while(0) + +/* Update the id, assert the proper rc, and expect a resulting fsm inst name + lookup */ +#define test_id(new_id, expect_rc, expect_name_suffix) do { \ + int rc; \ + fprintf(stderr, "osmo_fsm_inst_update_id(%s)\n", osmo_quote_str(new_id, -1)); \ + rc = osmo_fsm_inst_update_id(fi, new_id); \ + 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) + +/* Successfully set a new id, along with name and id lookup assertions */ +#define change_id(new_id) \ + test_id(new_id, 0, "(" new_id ")"); \ + assert_id(new_id) + + /* allocate FSM instance without id, there should be a name without id */ + fi = osmo_fsm_inst_alloc(&fsm, g_ctx, NULL, LOGL_DEBUG, NULL); + OSMO_ASSERT(fi); + /* CURRENT BUG: here I want to just do + assert_name("Test_FSM"); + * but when allocated with a NULL id, the fsm's name remains unset. Hence: */ + { + const char *expected_name = "Test_FSM"; + const char *name = osmo_fsm_inst_name(fi); + fprintf(stderr, " osmo_fsm_inst_name() == %s\n", osmo_quote_str(name, -1)); + if (safe_strcmp(name, expected_name)) { + fprintf(stderr, " ERROR: expected %s\n", osmo_quote_str(expected_name, -1)); + OSMO_ASSERT(false); + } + OSMO_ASSERT(osmo_fsm_inst_find_by_name(&fsm, "Test_FSM") == NULL); /* <- ERROR */ + fprintf(stderr, " osmo_fsm_inst_find_by_name(%s) == NULL\n", osmo_quote_str(expected_name, -1)); + } + + change_id("my_id"); + change_id("another_id"); + + test_id(NULL, 0, ""); + /* clear already cleared id */ + test_id(NULL, 0, ""); + + change_id("arbitrary_id"); + + /* clear id by empty string doesn't work */ + test_id("", -EINVAL, "(arbitrary_id)"); + + test_id("invalid.id", -EINVAL, "(arbitrary_id)"); + + fprintf(stderr, "\n--- %s() done\n\n", __func__); + + osmo_fsm_inst_term(fi, OSMO_FSM_TERM_REQUEST, NULL); +} + static const struct log_info_cat default_categories[] = { [DMAIN] = { .name = "DMAIN", @@ -198,17 +288,14 @@ OSMO_ASSERT(osmo_fsm_inst_find_by_name(&fsm, "my_id") == NULL); finst = foo(); - OSMO_ASSERT(osmo_fsm_inst_find_by_id(&fsm, "my_id") == finst); - OSMO_ASSERT(osmo_fsm_inst_find_by_name(&fsm, "Test_FSM(my_id)") == finst); - OSMO_ASSERT(osmo_fsm_inst_update_id(finst, "another_id") == 0); - OSMO_ASSERT(osmo_fsm_inst_find_by_id(&fsm, "another_id") == finst); - OSMO_ASSERT(osmo_fsm_inst_find_by_name(&fsm, "Test_FSM(another_id)") == finst); - OSMO_ASSERT(osmo_fsm_inst_update_id(finst, "my_id") == 0); while (main_loop_run) { osmo_select_main(0); } osmo_fsm_inst_free(finst); + + test_id_api(); + osmo_fsm_unregister(&fsm); exit(0); } diff --git a/tests/fsm/fsm_test.err b/tests/fsm/fsm_test.err index facc9f7..bc159bd 100644 --- a/tests/fsm/fsm_test.err +++ b/tests/fsm/fsm_test.err @@ -9,4 +9,48 @@ [0;mTest_FSM(my_id){TWO}: Timeout of T2342 [0;mTimer [0;mTest_FSM(my_id){TWO}: Deallocated +[0;m +--- test_id_api() +Test_FSM{NULL}: Allocated +[0;m osmo_fsm_inst_name() == "Test_FSM" + osmo_fsm_inst_find_by_name("Test_FSM") == NULL +osmo_fsm_inst_update_id("my_id") + rc == 0, ok + osmo_fsm_inst_name() == "Test_FSM(my_id)" + osmo_fsm_inst_find_by_name("Test_FSM(my_id)") == fi + osmo_fsm_inst_find_by_id("my_id") == fi +osmo_fsm_inst_update_id("another_id") + rc == 0, ok + osmo_fsm_inst_name() == "Test_FSM(another_id)" + osmo_fsm_inst_find_by_name("Test_FSM(another_id)") == fi + osmo_fsm_inst_find_by_id("another_id") == fi +osmo_fsm_inst_update_id(NULL) + rc == 0, ok + osmo_fsm_inst_name() == "Test_FSM" + osmo_fsm_inst_find_by_name("Test_FSM") == fi +osmo_fsm_inst_update_id(NULL) + rc == 0, ok + osmo_fsm_inst_name() == "Test_FSM" + osmo_fsm_inst_find_by_name("Test_FSM") == fi +osmo_fsm_inst_update_id("arbitrary_id") + rc == 0, ok + osmo_fsm_inst_name() == "Test_FSM(arbitrary_id)" + osmo_fsm_inst_find_by_name("Test_FSM(arbitrary_id)") == fi + osmo_fsm_inst_find_by_id("arbitrary_id") == fi +osmo_fsm_inst_update_id("") +Attempting to allocate FSM instance of type 'Test_FSM' with illegal identifier '' +[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("invalid.id") +Attempting to allocate FSM instance of type 'Test_FSM' with illegal identifier 'invalid.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 + +--- test_id_api() done + +Test_FSM(arbitrary_id){NULL}: Terminating (cause = OSMO_FSM_TERM_REQUEST) +[0;mTest_FSM(arbitrary_id){NULL}: Freeing instance +[0;mTest_FSM(arbitrary_id){NULL}: Deallocated [0;m \ No newline at end of file -- To view, visit https://gerrit.osmocom.org/7681 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I646ed918576ce196c395dc5f42a1507c52ace2c5 Gerrit-PatchSet: 2 Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder