Change in libosmocore[master]: fix comment for osmo_tdef_fsm_inst_state_chg default_timeout

pespin gerrit-no-reply at lists.osmocom.org
Wed Apr 28 10:34:18 UTC 2021


pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/23925 )

Change subject: fix comment for osmo_tdef_fsm_inst_state_chg default_timeout
......................................................................


Patch Set 2: Code-Review-1

(1 comment)

https://gerrit.osmocom.org/c/libosmocore/+/23925/2/include/osmocom/core/tdef.h 
File include/osmocom/core/tdef.h:

https://gerrit.osmocom.org/c/libosmocore/+/23925/2/include/osmocom/core/tdef.h@163 
PS2, Line 163: int _osmo_tdef_fsm_inst_state_chg(struct osmo_fsm_inst *fi, uint32_t state,
_osmo_tdef_fsm_inst_state_chg calls:
"osmo_tdef_get(tdefs, t->T, OSMO_TDEF_S, default_timeout);"

which contains:
"""
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);
}
"""

So yes, it asserts on -1 being passed. It's even documented there, and there's plenty of uses.
So in summary, if someone passes "-1" to osmo_tdef_fsm_inst_state_chg, it will ASSERT if the timer is not found, which is the expected behavior.

So please, don't drop that comment/use case. If at all, feel free to express it as "-1 casted to an unsigned long" or ULONG_MAX or whatever.



-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/23925
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I51b9172603984839448346c9836e43c8c802fcf8
Gerrit-Change-Number: 23925
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Wed, 28 Apr 2021 10:34:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210428/f018a76a/attachment.htm>


More information about the gerrit-log mailing list