<p>pespin has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/15546">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">tdef: Introduce min_val and max_val fields<br><br>This is useful for timers expected to have a range of valid or expected<br>values.<br><br>Validation is done at runtime when timer values are set by the app or by<br>the user through the VTY.<br><br>Related: OS#4190<br>Change-Id: I4661ac41c29a009a1d5fc57d87aaee6041c7d1b2<br>---<br>M include/osmocom/core/tdef.h<br>M src/tdef.c<br>M src/vty/tdef_vty.c<br>M tests/tdef/tdef_test.c<br>M tests/tdef/tdef_test.ok<br>5 files changed, 87 insertions(+), 12 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/46/15546/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmocom/core/tdef.h b/include/osmocom/core/tdef.h</span><br><span>index 8155688..ac4b640 100644</span><br><span>--- a/include/osmocom/core/tdef.h</span><br><span>+++ b/include/osmocom/core/tdef.h</span><br><span>@@ -77,6 +77,10 @@</span><br><span> /*! Currently active timeout value, e.g. set by user config. This is the only mutable member: a user may</span><br><span> * configure the timeout value, but neither unit nor any other field. */</span><br><span> unsigned long val;</span><br><span style="color: hsl(120, 100%, 40%);">+ /*! Minimum timer value (in this tdef unit), checked if set (not zero). */</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned long min_val;</span><br><span style="color: hsl(120, 100%, 40%);">+ /*! Maximum timer value (in this tdef unit), checked if set (not zero). */</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned long max_val;</span><br><span> };</span><br><span> </span><br><span> /*! Iterate an array of struct osmo_tdef, the last item should be fully zero, i.e. "{}".</span><br><span>@@ -98,6 +102,7 @@</span><br><span> long val_if_not_present);</span><br><span> struct osmo_tdef *osmo_tdef_get_entry(struct osmo_tdef *tdefs, int T);</span><br><span> int osmo_tdef_set(struct osmo_tdef *tdefs, int T, unsigned long val, enum osmo_tdef_unit val_unit);</span><br><span style="color: hsl(120, 100%, 40%);">+bool osmo_tdef_val_in_range(struct osmo_tdef *tdef, unsigned long new_val);</span><br><span> </span><br><span> /*! Using osmo_tdef for osmo_fsm_inst: array entry for a mapping of state numbers to timeout definitions.</span><br><span> * For a usage example, see osmo_tdef_get_state_timeout() and test_tdef_state_timeout() in tdef_test.c. */</span><br><span>diff --git a/src/tdef.c b/src/tdef.c</span><br><span>index ab6a51b..bb73f76 100644</span><br><span>--- a/src/tdef.c</span><br><span>+++ b/src/tdef.c</span><br><span>@@ -142,8 +142,12 @@</span><br><span> void osmo_tdefs_reset(struct osmo_tdef *tdefs)</span><br><span> {</span><br><span> struct osmo_tdef *t;</span><br><span style="color: hsl(0, 100%, 40%);">- osmo_tdef_for_each(t, tdefs)</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_tdef_for_each(t, tdefs) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!osmo_tdef_val_in_range(t, t->default_val))</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_panic("%s:%d Invalid default value %lu <= %lu <= %lu set in struct osmo_tdef (%s)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ __FILE__, __LINE__, t->min_val, t->default_val, t->max_val ? : (unsigned long) -1, t->desc);</span><br><span> t->val = t->default_val;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> /*! Return the value of a T timer from a list of osmo_tdef, in the given unit.</span><br><span>@@ -221,13 +225,29 @@</span><br><span> */</span><br><span> int osmo_tdef_set(struct osmo_tdef *tdefs, int T, unsigned long val, enum osmo_tdef_unit val_unit)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned long new_val;</span><br><span> struct osmo_tdef *t = osmo_tdef_get_entry(tdefs, T);</span><br><span> if (!t)</span><br><span> return -EEXIST;</span><br><span style="color: hsl(0, 100%, 40%);">- t->val = osmo_tdef_round(val, val_unit, t->unit);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ new_val = osmo_tdef_round(val, val_unit, t->unit);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!osmo_tdef_val_in_range(t, new_val))</span><br><span style="color: hsl(120, 100%, 40%);">+ return -EINVAL;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ t->val = new_val;</span><br><span> return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/*! Check if value new_val is in range of valid possible values for timer entry tdef.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] tdef Timer entry from a timer definition table.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] new_val The value whose validity to check, in units as per this timer entry.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \return true if inside range, false otherwise.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+bool osmo_tdef_val_in_range(struct osmo_tdef *tdef, unsigned long new_val)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ return new_val >= tdef->min_val && (!tdef->max_val || new_val <= tdef->max_val);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*! Using osmo_tdef for osmo_fsm_inst: find a given state's osmo_tdef_state_timeout entry.</span><br><span> *</span><br><span> * The timeouts_array shall contain exactly 32 elements, regardless whether only some of them are actually populated</span><br><span>diff --git a/src/vty/tdef_vty.c b/src/vty/tdef_vty.c</span><br><span>index eb05c3c..a8e48d3 100644</span><br><span>--- a/src/vty/tdef_vty.c</span><br><span>+++ b/src/vty/tdef_vty.c</span><br><span>@@ -115,12 +115,20 @@</span><br><span> */</span><br><span> int osmo_tdef_vty_set_cmd(struct vty *vty, struct osmo_tdef *tdefs, const char **args)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned long new_val;</span><br><span> const char *T_arg = args[0];</span><br><span> const char *val_arg = args[1];</span><br><span> struct osmo_tdef *t = osmo_tdef_vty_parse_T_arg(vty, tdefs, T_arg);</span><br><span> if (!t)</span><br><span> return CMD_WARNING;</span><br><span style="color: hsl(0, 100%, 40%);">- t->val = osmo_tdef_vty_parse_val_arg(val_arg, t->default_val);</span><br><span style="color: hsl(120, 100%, 40%);">+ new_val = osmo_tdef_vty_parse_val_arg(val_arg, t->default_val);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!osmo_tdef_val_in_range(t, new_val)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, "%% Invalid T timer value %lu (should be %lu <= val <= %lu)%s",</span><br><span style="color: hsl(120, 100%, 40%);">+ new_val, t->min_val, t->max_val ? : (unsigned long) -1 , VTY_NEWLINE);</span><br><span style="color: hsl(120, 100%, 40%);">+ return CMD_WARNING;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ t->val = new_val;</span><br><span> return CMD_SUCCESS;</span><br><span> }</span><br><span> </span><br><span>@@ -167,12 +175,19 @@</span><br><span> }</span><br><span> if (prefix_fmt)</span><br><span> vty_out_va(vty, prefix_fmt, va);</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, OSMO_T_FMT " = %lu%s%s\t%s (default: %lu%s%s)%s",</span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_T_FMT_ARGS(t->T), t->val,</span><br><span style="color: hsl(0, 100%, 40%);">- t->unit == OSMO_TDEF_CUSTOM ? "" : " ", t->unit == OSMO_TDEF_CUSTOM ? "" : osmo_tdef_unit_name(t->unit),</span><br><span style="color: hsl(0, 100%, 40%);">- t->desc, t->default_val,</span><br><span style="color: hsl(0, 100%, 40%);">- t->unit == OSMO_TDEF_CUSTOM ? "" : " ", t->unit == OSMO_TDEF_CUSTOM ? "" : osmo_tdef_unit_name(t->unit),</span><br><span style="color: hsl(0, 100%, 40%);">- VTY_NEWLINE);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, OSMO_T_FMT " = %lu", OSMO_T_FMT_ARGS(t->T), t->val);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (t->unit != OSMO_TDEF_CUSTOM)</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, " %s", osmo_tdef_unit_name(t->unit));</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, "\t%s (default: %lu", t->desc, t->default_val);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (t->unit != OSMO_TDEF_CUSTOM)</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, " %s", osmo_tdef_unit_name(t->unit));</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ if (t->min_val || t->max_val)</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, ", range: [%lu,%lu]", t->min_val, t->max_val ? : (unsigned long) -1);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, ")%s", VTY_NEWLINE);</span><br><span> }</span><br><span> </span><br><span> /*! Write to VTY the current status of one timer.</span><br><span>diff --git a/tests/tdef/tdef_test.c b/tests/tdef/tdef_test.c</span><br><span>index 12ca802..c672121 100644</span><br><span>--- a/tests/tdef/tdef_test.c</span><br><span>+++ b/tests/tdef/tdef_test.c</span><br><span>@@ -41,7 +41,7 @@</span><br><span> { .T=3, .default_val=100, .unit=OSMO_TDEF_M, .desc="100m" },</span><br><span> { .T=4, .default_val=100, .unit=OSMO_TDEF_CUSTOM, .desc="100 potatoes" },</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- { .T=7, .default_val=50, .desc="Water Boiling Timeout" }, // default is .unit=OSMO_TDEF_S == 0</span><br><span style="color: hsl(120, 100%, 40%);">+ { .T=7, .default_val=50, .desc="Water Boiling Timeout", .min_val=20, .max_val=800 }, // default is .unit=OSMO_TDEF_S == 0</span><br><span> { .T=8, .default_val=300, .desc="Tea brewing" },</span><br><span> { .T=9, .default_val=5, .unit=OSMO_TDEF_M, .desc="Let tea cool down before drinking" },</span><br><span> { .T=10, .default_val=20, .unit=OSMO_TDEF_M, .desc="Forgot to drink tea while it's warm" },</span><br><span>@@ -143,8 +143,9 @@</span><br><span> struct osmo_tdef *t;</span><br><span> printf("\n%s()\n", __func__);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- t = osmo_tdef_get_entry(tdefs, 7);</span><br><span> printf("setting 7 = 42\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ t = osmo_tdef_get_entry(tdefs, 7);</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(osmo_tdef_val_in_range(t, 42));</span><br><span> t->val = 42;</span><br><span> print_tdef_info(7);</span><br><span> print_tdef_get_short(tdefs, 7, OSMO_TDEF_MS);</span><br><span>@@ -153,13 +154,34 @@</span><br><span> print_tdef_get_short(tdefs, 7, OSMO_TDEF_CUSTOM);</span><br><span> </span><br><span> printf("setting 7 = 420\n");</span><br><span style="color: hsl(0, 100%, 40%);">- t->val = 420;</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(osmo_tdef_set(tdefs, 7, 420, OSMO_TDEF_S) == 0);</span><br><span> print_tdef_info(7);</span><br><span> print_tdef_get_short(tdefs, 7, OSMO_TDEF_MS);</span><br><span> print_tdef_get_short(tdefs, 7, OSMO_TDEF_S);</span><br><span> print_tdef_get_short(tdefs, 7, OSMO_TDEF_M);</span><br><span> print_tdef_get_short(tdefs, 7, OSMO_TDEF_CUSTOM);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ printf("setting 7 = 10 (EINVAL)\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(!osmo_tdef_val_in_range(t, 10));</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(osmo_tdef_set(tdefs, 7, 10, OSMO_TDEF_S) == -EINVAL);</span><br><span style="color: hsl(120, 100%, 40%);">+ print_tdef_info(7);</span><br><span style="color: hsl(120, 100%, 40%);">+ print_tdef_get_short(tdefs, 7, OSMO_TDEF_MS);</span><br><span style="color: hsl(120, 100%, 40%);">+ print_tdef_get_short(tdefs, 7, OSMO_TDEF_S);</span><br><span style="color: hsl(120, 100%, 40%);">+ print_tdef_get_short(tdefs, 7, OSMO_TDEF_M);</span><br><span style="color: hsl(120, 100%, 40%);">+ print_tdef_get_short(tdefs, 7, OSMO_TDEF_CUSTOM);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ printf("setting 7 = 900 (EINVAL)\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(!osmo_tdef_val_in_range(t, 900));</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(osmo_tdef_set(tdefs, 7, 900, OSMO_TDEF_S) == -EINVAL);</span><br><span style="color: hsl(120, 100%, 40%);">+ print_tdef_info(7);</span><br><span style="color: hsl(120, 100%, 40%);">+ print_tdef_get_short(tdefs, 7, OSMO_TDEF_MS);</span><br><span style="color: hsl(120, 100%, 40%);">+ print_tdef_get_short(tdefs, 7, OSMO_TDEF_S);</span><br><span style="color: hsl(120, 100%, 40%);">+ print_tdef_get_short(tdefs, 7, OSMO_TDEF_M);</span><br><span style="color: hsl(120, 100%, 40%);">+ print_tdef_get_short(tdefs, 7, OSMO_TDEF_CUSTOM);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ printf("setting 23 = 50 (EEXIST)\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_ASSERT(osmo_tdef_set(tdefs, 23, 50, OSMO_TDEF_S) == -EEXIST);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> printf("resetting\n");</span><br><span> osmo_tdefs_reset(tdefs);</span><br><span> print_tdef_info(7);</span><br><span>diff --git a/tests/tdef/tdef_test.ok b/tests/tdef/tdef_test.ok</span><br><span>index d9ef99b..a0bc50d 100644</span><br><span>--- a/tests/tdef/tdef_test.ok</span><br><span>+++ b/tests/tdef/tdef_test.ok</span><br><span>@@ -105,6 +105,19 @@</span><br><span> osmo_tdef_get(7, s) = 420</span><br><span> osmo_tdef_get(7, m) = 7</span><br><span> osmo_tdef_get(7, custom-unit) = 420</span><br><span style="color: hsl(120, 100%, 40%);">+setting 7 = 10 (EINVAL)</span><br><span style="color: hsl(120, 100%, 40%);">+T7=420s(def=50)</span><br><span style="color: hsl(120, 100%, 40%);">+osmo_tdef_get(7, ms) = 420000</span><br><span style="color: hsl(120, 100%, 40%);">+osmo_tdef_get(7, s) = 420</span><br><span style="color: hsl(120, 100%, 40%);">+osmo_tdef_get(7, m) = 7</span><br><span style="color: hsl(120, 100%, 40%);">+osmo_tdef_get(7, custom-unit) = 420</span><br><span style="color: hsl(120, 100%, 40%);">+setting 7 = 900 (EINVAL)</span><br><span style="color: hsl(120, 100%, 40%);">+T7=420s(def=50)</span><br><span style="color: hsl(120, 100%, 40%);">+osmo_tdef_get(7, ms) = 420000</span><br><span style="color: hsl(120, 100%, 40%);">+osmo_tdef_get(7, s) = 420</span><br><span style="color: hsl(120, 100%, 40%);">+osmo_tdef_get(7, m) = 7</span><br><span style="color: hsl(120, 100%, 40%);">+osmo_tdef_get(7, custom-unit) = 420</span><br><span style="color: hsl(120, 100%, 40%);">+setting 23 = 50 (EEXIST)</span><br><span> resetting</span><br><span> T7=50s</span><br><span> osmo_tdef_get(7, s) = 50</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/15546">change 15546</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/libosmocore/+/15546"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I4661ac41c29a009a1d5fc57d87aaee6041c7d1b2 </div>
<div style="display:none"> Gerrit-Change-Number: 15546 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>