libosmocore[master]: fsm: Add a function to change the FSM instance ID later

Harald Welte gerrit-no-reply at lists.osmocom.org
Thu Feb 8 09:22:38 UTC 2018


Patch Set 1: Code-Review-1

(2 comments)

https://gerrit.osmocom.org/#/c/6316/1/include/osmocom/core/fsm.h
File include/osmocom/core/fsm.h:

Line 90: 	char *id;
hm. We cannot change a "const char *" pointer?  I always thought it states that the destination buffer is 'const' and not that the pointer itself is 'const'.  But well, if it cannot be solved in a different way, we'll have to go ahead with this :/


https://gerrit.osmocom.org/#/c/6316/1/src/fsm.c
File src/fsm.c:

Line 205: bool osmo_fsm_inst_update_id(struct osmo_fsm_inst *fi, const char *id)
in general we return 0 on success and negative on error.  Only predicate-type functions like "is_this_a_valid_name()" deviate from this.  Your function breaks that general assumption, and it is difficult as a caller to know if one should check for negative returns or false, which is an easy way to introduce subtle code bugs.  Please return 0 on success and -EINVAL or the like on error.


-- 
To view, visit https://gerrit.osmocom.org/6316
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic216e5b11d4440f8e106a297714f4f06c1152945
Gerrit-PatchSet: 1
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: daniel <dwillmann at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-HasComments: Yes


More information about the gerrit-log mailing list