diff options
authorNeels Hofmeyr <neels@hofmeyr.de>2019-08-15 02:52:55 +0200
committerNeels Hofmeyr <neels@hofmeyr.de>2019-08-15 03:02:34 +0200
commit989f01c4065d3d3f83562b20a2a2b79189cb81f2 (patch)
parentd0b3b9edac978c91bf84aa2537aa24426685b1fb (diff)
osmo_tdef_get(): allow passing -1 as default timeoutneels/tdef
The intention of osmo_tdef_get()'s val_if_not_present argument was to return a default timeout, or to optionally abort the program for missing timer definitions if the default timeout is < 0. This was the case in the original implementation of this API in osmo-bsc, but in the migration to libosmocore, the argument was by accident changed to an unsigned type. In consequence, the assertion in the implementation that was intended to abort the program seemed bogus to coverity, and was fixed by removal in I7a544d2d43b83135def296674f777e48fe5fd80a -- the wrong direction, as is obvious from the API doc for osmo_tdef_get(). Note that osmo-bsc master passes -1 in various places and expects the program-abort behavior that was missing from the libosmocore implementation. Change the val_if_not_present argument to a signed type, and revert removal of the assertion, so that passing -1 has the effect described in the API doc: program abort on missing timer definition. This bug was not detected because it is hard to write tests that expect a program abort to happen, hence no tests for this API feature exist. Related: OS#4152 Change-Id: Ie61c3c85069916336e6dbd91a2c16f7634816417
3 files changed, 4 insertions, 2 deletions
index 8ccfa491..665ecf79 100644
@@ -7,3 +7,4 @@
# If any interfaces have been added since the last public release: c:r:a + 1.
# 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
diff --git a/include/osmocom/core/tdef.h b/include/osmocom/core/tdef.h
index c8d9053d..566f5dd3 100644
--- a/include/osmocom/core/tdef.h
+++ b/include/osmocom/core/tdef.h
@@ -95,7 +95,7 @@ struct osmo_tdef {
void osmo_tdefs_reset(struct osmo_tdef *tdefs);
unsigned long osmo_tdef_get(const struct osmo_tdef *tdefs, int T, enum osmo_tdef_unit as_unit,
- unsigned long val_if_not_present);
+ long val_if_not_present);
struct osmo_tdef *osmo_tdef_get_entry(struct osmo_tdef *tdefs, int T);
/*! Using osmo_tdef for osmo_fsm_inst: array entry for a mapping of state numbers to timeout definitions.
diff --git a/src/tdef.c b/src/tdef.c
index 3cfb17c0..40a99002 100644
--- a/src/tdef.c
+++ b/src/tdef.c
@@ -183,10 +183,11 @@ void osmo_tdefs_reset(struct osmo_tdef *tdefs)
* \param[in] val_if_not_present Fallback value to return if no timeout is defined.
* \return Timeout value in the unit given by as_unit, rounded up if necessary, or val_if_not_present.
-unsigned long osmo_tdef_get(const struct osmo_tdef *tdefs, int T, enum osmo_tdef_unit as_unit, unsigned long val_if_not_present)
+unsigned long osmo_tdef_get(const struct osmo_tdef *tdefs, int T, enum osmo_tdef_unit as_unit, long val_if_not_present)
const struct osmo_tdef *t = osmo_tdef_get_entry((struct osmo_tdef*)tdefs, T);
if (!t) {
+ OSMO_ASSERT(val_if_not_present >= 0);
return val_if_not_present;
return osmo_tdef_round(t->val, t->unit, as_unit);