From 0cbe8f01003ca71209c12eacac3abaeb9a7df862 Mon Sep 17 00:00:00 2001 From: Pau Espin Pedrol Date: Tue, 17 Sep 2019 13:13:52 +0200 Subject: tdef: Introduce min_val and max_val fields This is useful for timers expected to have a range of valid or expected values. Validation is done at runtime when timer values are set by the app or by the user through the VTY. Related: OS#4190 Change-Id: I4661ac41c29a009a1d5fc57d87aaee6041c7d1b2 --- TODO-RELEASE | 1 + include/osmocom/core/tdef.h | 6 ++++ src/tdef.c | 56 ++++++++++++++++++++++++++++++-- src/vty/tdef_vty.c | 35 ++++++++++++++++---- tests/tdef/tdef_test.c | 25 ++++++++++++-- tests/tdef/tdef_test.ok | 12 +++++++ tests/tdef/tdef_vty_test_config_root.c | 3 ++ tests/tdef/tdef_vty_test_config_root.vty | 50 ++++++++++++++++++++++++++++ 8 files changed, 175 insertions(+), 13 deletions(-) diff --git a/TODO-RELEASE b/TODO-RELEASE index 665ecf79..547b5a90 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -8,3 +8,4 @@ # If any interfaces have been removed or changed since the last public release: c:r:0. #library what description / commit summary line core osmo_tdef_get() change val_if_not_present arg from unsigned long to long to allow passing -1 +core struct osmo_tdef fields min_val,max_val added, ABI break (arrays of structs used in programs) diff --git a/include/osmocom/core/tdef.h b/include/osmocom/core/tdef.h index 81556883..54819d95 100644 --- a/include/osmocom/core/tdef.h +++ b/include/osmocom/core/tdef.h @@ -77,6 +77,10 @@ struct osmo_tdef { /*! Currently active timeout value, e.g. set by user config. This is the only mutable member: a user may * configure the timeout value, but neither unit nor any other field. */ unsigned long val; + /*! Minimum timer value (in this tdef unit), checked if set (not zero). */ + unsigned long min_val; + /*! Maximum timer value (in this tdef unit), checked if set (not zero). */ + unsigned long max_val; }; /*! Iterate an array of struct osmo_tdef, the last item should be fully zero, i.e. "{}". @@ -98,6 +102,8 @@ unsigned long osmo_tdef_get(const struct osmo_tdef *tdefs, int T, enum osmo_tdef long val_if_not_present); struct osmo_tdef *osmo_tdef_get_entry(struct osmo_tdef *tdefs, int T); int osmo_tdef_set(struct osmo_tdef *tdefs, int T, unsigned long val, enum osmo_tdef_unit val_unit); +bool osmo_tdef_val_in_range(struct osmo_tdef *tdef, unsigned long new_val); +int osmo_tdef_range_str_buf(char *buf, size_t buf_len, struct osmo_tdef *t); /*! Using osmo_tdef for osmo_fsm_inst: array entry for a mapping of state numbers to timeout definitions. * For a usage example, see osmo_tdef_get_state_timeout() and test_tdef_state_timeout() in tdef_test.c. */ diff --git a/src/tdef.c b/src/tdef.c index ab6a51b4..94d987fa 100644 --- a/src/tdef.c +++ b/src/tdef.c @@ -136,14 +136,22 @@ static unsigned long osmo_tdef_round(unsigned long val, enum osmo_tdef_unit from /*! Set all osmo_tdef values to the default_val. * It is convenient to define a tdefs array by setting only the default_val, and calling osmo_tdefs_reset() once for - * program startup. (See also osmo_tdef_vty_init()) + * program startup. (See also osmo_tdef_vty_init()). + * During call to this function, default values are verified to be inside valid range; process is aborted otherwise. * \param[in] tdefs Array of timer definitions, last entry being fully zero. */ void osmo_tdefs_reset(struct osmo_tdef *tdefs) { struct osmo_tdef *t; - osmo_tdef_for_each(t, tdefs) + osmo_tdef_for_each(t, tdefs) { + if (!osmo_tdef_val_in_range(t, t->default_val)) { + char range_str[64]; + osmo_tdef_range_str_buf(range_str, sizeof(range_str), t); + osmo_panic("%s:%d Timer " OSMO_T_FMT " contains default value %lu not in range %s\n", + __FILE__, __LINE__, OSMO_T_FMT_ARGS(t->T), t->default_val, range_str); + } t->val = t->default_val; + } } /*! Return the value of a T timer from a list of osmo_tdef, in the given unit. @@ -221,13 +229,55 @@ struct osmo_tdef *osmo_tdef_get_entry(struct osmo_tdef *tdefs, int T) */ int osmo_tdef_set(struct osmo_tdef *tdefs, int T, unsigned long val, enum osmo_tdef_unit val_unit) { + unsigned long new_val; struct osmo_tdef *t = osmo_tdef_get_entry(tdefs, T); if (!t) return -EEXIST; - t->val = osmo_tdef_round(val, val_unit, t->unit); + + new_val = osmo_tdef_round(val, val_unit, t->unit); + if (!osmo_tdef_val_in_range(t, new_val)) + return -ERANGE; + + t->val = new_val; return 0; } +/*! Check if value new_val is in range of valid possible values for timer entry tdef. + * \param[in] tdef Timer entry from a timer definition table. + * \param[in] new_val The value whose validity to check, in units as per this timer entry. + * \return true if inside range, false otherwise. + */ +bool osmo_tdef_val_in_range(struct osmo_tdef *tdef, unsigned long new_val) +{ + return new_val >= tdef->min_val && (!tdef->max_val || new_val <= tdef->max_val); +} + +/*! Write string representation of osmo_tdef range into buf. + * \param[in] buf The buffer where the string representation is stored. + * \param[in] buf_len Length of buffer in bytes. + * \param[in] tdef Timer entry from a timer definition table. + * \return The number of characters printed on success, negative on error. See snprintf(). + */ +int osmo_tdef_range_str_buf(char *buf, size_t buf_len, struct osmo_tdef *t) +{ + int ret, len = 0, offset = 0, rem = buf_len; + + buf[0] = '\0'; + ret = snprintf(buf + offset, rem, "[%lu .. ", t->min_val); + if (ret < 0) + return ret; + OSMO_SNPRINTF_RET(ret, rem, offset, len); + + if (t->max_val) + ret = snprintf(buf + offset, rem, "%lu]", t->max_val); + else + ret = snprintf(buf + offset, rem, "inf]"); + if (ret < 0) + return ret; + OSMO_SNPRINTF_RET(ret, rem, offset, len); + return ret; +} + /*! Using osmo_tdef for osmo_fsm_inst: find a given state's osmo_tdef_state_timeout entry. * * The timeouts_array shall contain exactly 32 elements, regardless whether only some of them are actually populated diff --git a/src/vty/tdef_vty.c b/src/vty/tdef_vty.c index eb05c3c5..4549a61c 100644 --- a/src/vty/tdef_vty.c +++ b/src/vty/tdef_vty.c @@ -115,12 +115,22 @@ unsigned long osmo_tdef_vty_parse_val_arg(const char *val_arg, unsigned long def */ int osmo_tdef_vty_set_cmd(struct vty *vty, struct osmo_tdef *tdefs, const char **args) { + unsigned long new_val; const char *T_arg = args[0]; const char *val_arg = args[1]; struct osmo_tdef *t = osmo_tdef_vty_parse_T_arg(vty, tdefs, T_arg); if (!t) return CMD_WARNING; - t->val = osmo_tdef_vty_parse_val_arg(val_arg, t->default_val); + new_val = osmo_tdef_vty_parse_val_arg(val_arg, t->default_val); + + if (!osmo_tdef_val_in_range(t, new_val)) { + char range_str[64]; + osmo_tdef_range_str_buf(range_str, sizeof(range_str), t); + vty_out(vty, "%% Timer " OSMO_T_FMT " value %lu is out of range %s%s", + OSMO_T_FMT_ARGS(t->T), new_val, range_str, VTY_NEWLINE); + return CMD_WARNING; + } + t->val = new_val; return CMD_SUCCESS; } @@ -161,18 +171,29 @@ int osmo_tdef_vty_show_cmd(struct vty *vty, struct osmo_tdef *tdefs, const char */ void osmo_tdef_vty_out_one_va(struct vty *vty, struct osmo_tdef *t, const char *prefix_fmt, va_list va) { + char range_str[64]; + if (!t) { vty_out(vty, "%% Error: no such timer%s", VTY_NEWLINE); return; } if (prefix_fmt) vty_out_va(vty, prefix_fmt, va); - vty_out(vty, OSMO_T_FMT " = %lu%s%s\t%s (default: %lu%s%s)%s", - OSMO_T_FMT_ARGS(t->T), t->val, - t->unit == OSMO_TDEF_CUSTOM ? "" : " ", t->unit == OSMO_TDEF_CUSTOM ? "" : osmo_tdef_unit_name(t->unit), - t->desc, t->default_val, - t->unit == OSMO_TDEF_CUSTOM ? "" : " ", t->unit == OSMO_TDEF_CUSTOM ? "" : osmo_tdef_unit_name(t->unit), - VTY_NEWLINE); + + vty_out(vty, OSMO_T_FMT " = %lu", OSMO_T_FMT_ARGS(t->T), t->val); + if (t->unit != OSMO_TDEF_CUSTOM) + vty_out(vty, " %s", osmo_tdef_unit_name(t->unit)); + + vty_out(vty, "\t%s (default: %lu", t->desc, t->default_val); + if (t->unit != OSMO_TDEF_CUSTOM) + vty_out(vty, " %s", osmo_tdef_unit_name(t->unit)); + + if (t->min_val || t->max_val) { + osmo_tdef_range_str_buf(range_str, sizeof(range_str), t); + vty_out(vty, ", range: %s", range_str); + } + + vty_out(vty, ")%s", VTY_NEWLINE); } /*! Write to VTY the current status of one timer. diff --git a/tests/tdef/tdef_test.c b/tests/tdef/tdef_test.c index 12ca8020..60066b1e 100644 --- a/tests/tdef/tdef_test.c +++ b/tests/tdef/tdef_test.c @@ -41,7 +41,7 @@ static struct osmo_tdef tdefs[] = { { .T=3, .default_val=100, .unit=OSMO_TDEF_M, .desc="100m" }, { .T=4, .default_val=100, .unit=OSMO_TDEF_CUSTOM, .desc="100 potatoes" }, - { .T=7, .default_val=50, .desc="Water Boiling Timeout" }, // default is .unit=OSMO_TDEF_S == 0 + { .T=7, .default_val=50, .desc="Water Boiling Timeout", .min_val=20, .max_val=800 }, // default is .unit=OSMO_TDEF_S == 0 { .T=8, .default_val=300, .desc="Tea brewing" }, { .T=9, .default_val=5, .unit=OSMO_TDEF_M, .desc="Let tea cool down before drinking" }, { .T=10, .default_val=20, .unit=OSMO_TDEF_M, .desc="Forgot to drink tea while it's warm" }, @@ -143,8 +143,9 @@ static void test_tdef_set_and_get() struct osmo_tdef *t; printf("\n%s()\n", __func__); - t = osmo_tdef_get_entry(tdefs, 7); printf("setting 7 = 42\n"); + t = osmo_tdef_get_entry(tdefs, 7); + OSMO_ASSERT(osmo_tdef_val_in_range(t, 42)); t->val = 42; print_tdef_info(7); print_tdef_get_short(tdefs, 7, OSMO_TDEF_MS); @@ -153,7 +154,25 @@ static void test_tdef_set_and_get() print_tdef_get_short(tdefs, 7, OSMO_TDEF_CUSTOM); printf("setting 7 = 420\n"); - t->val = 420; + OSMO_ASSERT(osmo_tdef_set(tdefs, 7, 420, OSMO_TDEF_S) == 0); + print_tdef_info(7); + print_tdef_get_short(tdefs, 7, OSMO_TDEF_MS); + print_tdef_get_short(tdefs, 7, OSMO_TDEF_S); + print_tdef_get_short(tdefs, 7, OSMO_TDEF_M); + print_tdef_get_short(tdefs, 7, OSMO_TDEF_CUSTOM); + + printf("setting 7 = 10 (ERANGE)\n"); + OSMO_ASSERT(!osmo_tdef_val_in_range(t, 10)); + OSMO_ASSERT(osmo_tdef_set(tdefs, 7, 10, OSMO_TDEF_S) == -ERANGE); + print_tdef_info(7); + print_tdef_get_short(tdefs, 7, OSMO_TDEF_MS); + print_tdef_get_short(tdefs, 7, OSMO_TDEF_S); + print_tdef_get_short(tdefs, 7, OSMO_TDEF_M); + print_tdef_get_short(tdefs, 7, OSMO_TDEF_CUSTOM); + + printf("setting 7 = 900 (ERANGE)\n"); + OSMO_ASSERT(!osmo_tdef_val_in_range(t, 900)); + OSMO_ASSERT(osmo_tdef_set(tdefs, 7, 900, OSMO_TDEF_S) == -ERANGE); print_tdef_info(7); print_tdef_get_short(tdefs, 7, OSMO_TDEF_MS); print_tdef_get_short(tdefs, 7, OSMO_TDEF_S); diff --git a/tests/tdef/tdef_test.ok b/tests/tdef/tdef_test.ok index d9ef99b2..2a3617e8 100644 --- a/tests/tdef/tdef_test.ok +++ b/tests/tdef/tdef_test.ok @@ -105,6 +105,18 @@ osmo_tdef_get(7, ms) = 420000 osmo_tdef_get(7, s) = 420 osmo_tdef_get(7, m) = 7 osmo_tdef_get(7, custom-unit) = 420 +setting 7 = 10 (ERANGE) +T7=420s(def=50) +osmo_tdef_get(7, ms) = 420000 +osmo_tdef_get(7, s) = 420 +osmo_tdef_get(7, m) = 7 +osmo_tdef_get(7, custom-unit) = 420 +setting 7 = 900 (ERANGE) +T7=420s(def=50) +osmo_tdef_get(7, ms) = 420000 +osmo_tdef_get(7, s) = 420 +osmo_tdef_get(7, m) = 7 +osmo_tdef_get(7, custom-unit) = 420 resetting T7=50s osmo_tdef_get(7, s) = 50 diff --git a/tests/tdef/tdef_vty_test_config_root.c b/tests/tdef/tdef_vty_test_config_root.c index d69e0285..92113e87 100644 --- a/tests/tdef/tdef_vty_test_config_root.c +++ b/tests/tdef/tdef_vty_test_config_root.c @@ -55,6 +55,9 @@ static struct osmo_tdef tdefs_test[] = { { .T=4, .default_val=100, .unit=OSMO_TDEF_CUSTOM, .desc="Testing a hundred potatoes" }, { .T=0x7fffffff, .default_val=0xffffffff, .unit=OSMO_TDEF_M, .desc="Very large" }, { .T=-23, .default_val=239471, .desc="Negative T number" }, + { .T=30, .default_val=50, .desc="Testing range min", .min_val=20 }, + { .T=31, .default_val=50, .desc="Testing range max", .max_val=52 }, + { .T=32, .default_val=50, .desc="Testing range both", .min_val=20, .max_val=52 }, {} // <-- important! last entry shall be zero }; diff --git a/tests/tdef/tdef_vty_test_config_root.vty b/tests/tdef/tdef_vty_test_config_root.vty index f3aba0f9..6a53b805 100644 --- a/tests/tdef/tdef_vty_test_config_root.vty +++ b/tests/tdef/tdef_vty_test_config_root.vty @@ -22,6 +22,9 @@ test: T3 = 100 m Testing a hundred minutes (default: 100 m) test: T4 = 100 Testing a hundred potatoes (default: 100) test: T2147483647 = 4294967295 m Very large (default: 4294967295 m) test: X23 = 239471 s Negative T number (default: 239471 s) +test: T30 = 50 s Testing range min (default: 50 s, range: [20 .. inf]) +test: T31 = 50 s Testing range max (default: 50 s, range: [0 .. 52]) +test: T32 = 50 s Testing range both (default: 50 s, range: [20 .. 52]) software: T1 = 30 m Write code (default: 30 m) software: T2 = 20 ms Hit segfault (default: 20 ms) software: T3 = 480 m Fix bugs (default: 480 m) @@ -38,6 +41,9 @@ test: T3 = 100 m Testing a hundred minutes (default: 100 m) test: T4 = 100 Testing a hundred potatoes (default: 100) test: T2147483647 = 4294967295 m Very large (default: 4294967295 m) test: X23 = 239471 s Negative T number (default: 239471 s) +test: T30 = 50 s Testing range min (default: 50 s, range: [20 .. inf]) +test: T31 = 50 s Testing range max (default: 50 s, range: [0 .. 52]) +test: T32 = 50 s Testing range both (default: 50 s, range: [20 .. 52]) software: T1 = 30 m Write code (default: 30 m) software: T2 = 20 ms Hit segfault (default: 20 ms) software: T3 = 480 m Fix bugs (default: 480 m) @@ -83,6 +89,9 @@ test: T3 = 100 m Testing a hundred minutes (default: 100 m) test: T4 = 100 Testing a hundred potatoes (default: 100) test: T2147483647 = 4294967295 m Very large (default: 4294967295 m) test: X23 = 239471 s Negative T number (default: 239471 s) +test: T30 = 50 s Testing range min (default: 50 s, range: [20 .. inf]) +test: T31 = 50 s Testing range max (default: 50 s, range: [0 .. 52]) +test: T32 = 50 s Testing range both (default: 50 s, range: [20 .. 52]) software: T1 = 30 m Write code (default: 30 m) software: T2 = 20 ms Hit segfault (default: 20 ms) software: T3 = 480 m Fix bugs (default: 480 m) @@ -167,6 +176,32 @@ tdef_vty_test(config)# timer tes T2 100 tdef_vty_test(config)# timer te T2 100 % Ambiguous command. +tdef_vty_test(config)# timer test 30 40 +tdef_vty_test(config)# timer test 30 60 +tdef_vty_test(config)# timer test 30 10 +% Timer T30 value 10 is out of range [20 .. inf] + +tdef_vty_test(config)# timer test 31 40 +tdef_vty_test(config)# timer test 31 60 +% Timer T31 value 60 is out of range [0 .. 52] +tdef_vty_test(config)# timer test 31 10 + +tdef_vty_test(config)# timer test 32 40 +tdef_vty_test(config)# timer test 32 60 +% Timer T32 value 60 is out of range [20 .. 52] +tdef_vty_test(config)# timer test 32 10 +% Timer T32 value 10 is out of range [20 .. 52] + +tdef_vty_test(config)# timer test +test: T1 = 100 s Testing a hundred seconds (default: 100 s) +test: T2 = 100 ms Testing a hundred milliseconds (default: 100 ms) +test: T3 = 100 m Testing a hundred minutes (default: 100 m) +test: T4 = 100 Testing a hundred potatoes (default: 100) +test: T2147483647 = 4294967295 m Very large (default: 4294967295 m) +test: X23 = 239471 s Negative T number (default: 239471 s) +test: T30 = 60 s Testing range min (default: 50 s, range: [20 .. inf]) +test: T31 = 10 s Testing range max (default: 50 s, range: [0 .. 52]) +test: T32 = 40 s Testing range both (default: 50 s, range: [20 .. 52]) tdef_vty_test(config)# do show timer software software: T1 = 30 m Write code (default: 30 m) @@ -230,6 +265,9 @@ test: T3 = 100 m Testing a hundred minutes (default: 100 m) test: T4 = 100 Testing a hundred potatoes (default: 100) test: T2147483647 = 4294967295 m Very large (default: 4294967295 m) test: X23 = 239471 s Negative T number (default: 239471 s) +test: T30 = 60 s Testing range min (default: 50 s, range: [20 .. inf]) +test: T31 = 10 s Testing range max (default: 50 s, range: [0 .. 52]) +test: T32 = 40 s Testing range both (default: 50 s, range: [20 .. 52]) software: T1 = 13 m Write code (default: 30 m) software: T2 = 0 ms Hit segfault (default: 20 ms) software: T3 = 23 m Fix bugs (default: 480 m) @@ -245,6 +283,9 @@ test: T3 = 100 m Testing a hundred minutes (default: 100 m) test: T4 = 100 Testing a hundred potatoes (default: 100) test: T2147483647 = 4294967295 m Very large (default: 4294967295 m) test: X23 = 239471 s Negative T number (default: 239471 s) +test: T30 = 60 s Testing range min (default: 50 s, range: [20 .. inf]) +test: T31 = 10 s Testing range max (default: 50 s, range: [0 .. 52]) +test: T32 = 40 s Testing range both (default: 50 s, range: [20 .. 52]) software: T1 = 13 m Write code (default: 30 m) software: T2 = 0 ms Hit segfault (default: 20 ms) software: T3 = 23 m Fix bugs (default: 480 m) @@ -252,6 +293,9 @@ software: T3 = 23 m Fix bugs (default: 480 m) tdef_vty_test(config)# show running-config ... !timer timer tea T3 32 +timer test T30 60 +timer test T31 10 +timer test T32 40 timer software T1 13 timer software T2 0 timer software T3 23 @@ -261,6 +305,9 @@ tdef_vty_test(config)# timer tea T3 default tdef_vty_test(config)# timer software T1 default tdef_vty_test(config)# show running-config ... !timer +timer test T30 60 +timer test T31 10 +timer test T32 40 timer software T2 0 timer software T3 23 ... !timer @@ -269,5 +316,8 @@ tdef_vty_test(config)# timer softw 2 default tdef_vty_test(config)# timer software 2 default tdef_vty_test(config)# show running-config ... !timer +timer test T30 60 +timer test T31 10 +timer test T32 40 timer software T3 23 ... !timer -- cgit v1.2.3