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/.
pespin gerrit-no-reply at lists.osmocom.orgpespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/15546 Change subject: tdef: Introduce min_val and max_val fields ...................................................................... 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 --- M include/osmocom/core/tdef.h M src/tdef.c M src/vty/tdef_vty.c M tests/tdef/tdef_test.c M tests/tdef/tdef_test.ok 5 files changed, 87 insertions(+), 12 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/46/15546/1 diff --git a/include/osmocom/core/tdef.h b/include/osmocom/core/tdef.h index 8155688..ac4b640 100644 --- a/include/osmocom/core/tdef.h +++ b/include/osmocom/core/tdef.h @@ -77,6 +77,10 @@ /*! 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,7 @@ 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); /*! 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 ab6a51b..bb73f76 100644 --- a/src/tdef.c +++ b/src/tdef.c @@ -142,8 +142,12 @@ 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)) + osmo_panic("%s:%d Invalid default value %lu <= %lu <= %lu set in struct osmo_tdef (%s)\n", + __FILE__, __LINE__, t->min_val, t->default_val, t->max_val ? : (unsigned long) -1, t->desc); t->val = t->default_val; + } } /*! Return the value of a T timer from a list of osmo_tdef, in the given unit. @@ -221,13 +225,29 @@ */ 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 -EINVAL; + + 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); +} + /*! 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 eb05c3c..a8e48d3 100644 --- a/src/vty/tdef_vty.c +++ b/src/vty/tdef_vty.c @@ -115,12 +115,20 @@ */ 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)) { + vty_out(vty, "%% Invalid T timer value %lu (should be %lu <= val <= %lu)%s", + new_val, t->min_val, t->max_val ? : (unsigned long) -1 , VTY_NEWLINE); + return CMD_WARNING; + } + t->val = new_val; return CMD_SUCCESS; } @@ -167,12 +175,19 @@ } 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) + vty_out(vty, ", range: [%lu,%lu]", t->min_val, t->max_val ? : (unsigned long) -1); + + 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 12ca802..c672121 100644 --- a/tests/tdef/tdef_test.c +++ b/tests/tdef/tdef_test.c @@ -41,7 +41,7 @@ { .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 @@ 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,13 +154,34 @@ 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 (EINVAL)\n"); + OSMO_ASSERT(!osmo_tdef_val_in_range(t, 10)); + OSMO_ASSERT(osmo_tdef_set(tdefs, 7, 10, OSMO_TDEF_S) == -EINVAL); + 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 (EINVAL)\n"); + OSMO_ASSERT(!osmo_tdef_val_in_range(t, 900)); + OSMO_ASSERT(osmo_tdef_set(tdefs, 7, 900, OSMO_TDEF_S) == -EINVAL); + 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 23 = 50 (EEXIST)\n"); + OSMO_ASSERT(osmo_tdef_set(tdefs, 23, 50, OSMO_TDEF_S) == -EEXIST); + printf("resetting\n"); osmo_tdefs_reset(tdefs); print_tdef_info(7); diff --git a/tests/tdef/tdef_test.ok b/tests/tdef/tdef_test.ok index d9ef99b..a0bc50d 100644 --- a/tests/tdef/tdef_test.ok +++ b/tests/tdef/tdef_test.ok @@ -105,6 +105,19 @@ osmo_tdef_get(7, s) = 420 osmo_tdef_get(7, m) = 7 osmo_tdef_get(7, custom-unit) = 420 +setting 7 = 10 (EINVAL) +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 (EINVAL) +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 23 = 50 (EEXIST) resetting T7=50s osmo_tdef_get(7, s) = 50 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/15546 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I4661ac41c29a009a1d5fc57d87aaee6041c7d1b2 Gerrit-Change-Number: 15546 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin at sysmocom.de> Gerrit-MessageType: newchange -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190917/8c6ce8fc/attachment.htm>