Change in ...osmo-msc[master]: Implements a global switch on the network to disable call waiting.

neels gerrit-no-reply at
Thu Aug 8 14:19:19 UTC 2019

neels has posted comments on this change. ( )

Change subject: Implements a global switch on the network to disable call waiting.

Patch Set 1: Code-Review-1


build failure: you need to adjust the vty transcript test that failed. You added the 'callwaiting' vty option, and the transcript test didn't expect that to appear. 
Commit Message: 
PS1, Line 22: of call waiting.
would be nice to briefly mention or reference issues of the unpredictable behavior 
File include/osmocom/msc/gsm_data.h: 
PS1, Line 258: 	uint8_t callwaiting;
rather use bool 
File include/osmocom/msc/transaction.h: 
PS1, Line 142: 					struct vlr_subscr *vsub);
Instead of adding this I would suggest the caller use

  trans_find_by_type(msc_a_for_vsub(vsub, true), TRANS_CC);

(to avoid API clutter) 
File src/libmsc/gsm_04_08_cc.c: 
PS1, Line 1806: 	struct gsm_trans *_trans = NULL;
> Not sure about this, but I got a segfault (in other code path) while testing, wasn't sure if it was  […]
as I read the code there shouldn't be a segfault, but it is better style to use a separate variable indeed.
Rather declare the separate variable at the start of the "if (!net->callwaiting)" scope because it isn't used elsewhere, and rather name it like existing_cc_trans or something to clarify. 
PS1, Line 1879: 		if (!net->callwaiting) {
(put the trans var decl here) 
PS1, Line 1880: 			_trans = trans_find_cc_by_vsub(net, vsub);
(use trans_find_by_type() here, see below) 
PS1, Line 1886: 					_trans->cc.state,
cosmetic: no need to log both the state number and the gsm48_cc_state_name(), just the name is enough 
PS1, Line 1894: 		if (!vsub->lu_complete) {
would make sense to keep this check before the callwaiting check. It's higher up the priority of reject reasons. so... (read below) 
PS1, Line 1902: 		}
<-- MARKER (see below) 
PS1, Line 1917: 		msc_a = msc_a_for_vsub(vsub, true);
and here is an msc_a already.

So I would suggest:

- move this msc_a = msc_a_for_vsub() up to "MARKER" above.
- put the net->callwaiting check right below that
- use trans_find_by_type(msc_a, TRANS_CC) instead of adding API 
File src/libmsc/msc_vty.c: 
PS1, Line 360: 		vty_out(vty, " callwaiting%s", VTY_NEWLINE);
(we often don't write back the default, so could only write 'no callwaiting' in case it is false)

To view, visit
To unsubscribe, or for help writing mail filters, visit

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I3eb6f23f7103e3002874fb5d3a30c9de952202ae
Gerrit-Change-Number: 15120
Gerrit-PatchSet: 1
Gerrit-Owner: keith <keith at>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: keith <keith at>
Gerrit-Reviewer: neels <nhofmeyr at>
Gerrit-Comment-Date: Thu, 08 Aug 2019 14:19:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: keith <keith at>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <>

More information about the gerrit-log mailing list