Change in ...libosmocore[master]: tdef: Introduce min_val and max_val fields

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.org
Tue Sep 17 11:15:49 UTC 2019 

pespin 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>

More information about the gerrit-log mailing list