Project

General

Profile

Actions

Bug #4152

closed

osmo_tdef_get() has API doc that encourages to pass -1 to an unsigned long argument, and promises certain behavior in case of -1 being passed

Added by neels over 4 years ago. Updated over 4 years ago.

Status:
Resolved
Priority:
High
Assignee:
Category:
-
Target version:
-
Start date:
08/15/2019
Due date:
% Done:

100%

Spec Reference:

Description

unsigned long osmo_tdef_get(const struct osmo_tdef *tdefs, int T, enum osmo_tdef_unit as_unit, unsigned long val_if_not_present)

says in the API doc

 *      val = osmo_tdef_get(global_T_defs, 99, OSMO_TDEF_S, -1); // not defined, program aborts!

That is neither implemented nor possible with an unsigned arg.
How did that get past our review...

I messed up there, how should we fix it?

Given that osmo-bsc has code passing -1, maybe we should make the argument signed?
And then we should indeed add the advertised behavior, i.e. a program abort if no timer is defined?

I'm pretty sure that was also actually implemented in an earlier version, but must have gotten lost in the changes and migration to libosmocore... :/

Noticed this when reading https://gerrit.osmocom.org/c/osmo-sgsn/+/15214
And noticed that osmo-bsc master passes -1 in seven distinct places.

Another way would be to fix the API doc and not abort the program on missing timer definitions,
but in fact I think that's a pretty nice API feature to have?

Actions

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)