Change in osmo-bsc[master]: LCLS: explicitly check for mode before closing the loop

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Mon Nov 5 03:02:37 UTC 2018


Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/11565 )

Change subject: LCLS: explicitly check for mode before closing the loop
......................................................................


Patch Set 1: Code-Review-1

(5 comments)

https://gerrit.osmocom.org/#/c/11565/1//COMMIT_MSG
Commit Message:

https://gerrit.osmocom.org/#/c/11565/1//COMMIT_MSG@10
PS1, Line 10: * explicitly check for MGW mode before closing the loop via MGCP MDCX
I don't see how these two changes end up in the same patch. Please separate orthogonal changes, especially when it's logging tweaks vs. functional changes


https://gerrit.osmocom.org/#/c/11565/1/include/osmocom/bsc/bsc_msc_data.h
File include/osmocom/bsc/bsc_msc_data.h:

https://gerrit.osmocom.org/#/c/11565/1/include/osmocom/bsc/bsc_msc_data.h@62
PS1, Line 62: extern const struct value_string bsc_lcls_mode_names[];
at some point we agreed to always add something like this:

  static inline const char *bsc_lcls_mode_name(enum bsc_lcls_mode val)
  { return get_value_string(...); }


https://gerrit.osmocom.org/#/c/11565/1/src/osmo-bsc/gsm_data.c
File src/osmo-bsc/gsm_data.c:

https://gerrit.osmocom.org/#/c/11565/1/src/osmo-bsc/gsm_data.c@53
PS1, Line 53: 	{ 0, NULL }
(possible to just write '{}' in the last line)


https://gerrit.osmocom.org/#/c/11565/1/src/osmo-bsc/osmo_bsc_lcls.c
File src/osmo-bsc/osmo_bsc_lcls.c:

https://gerrit.osmocom.org/#/c/11565/1/src/osmo-bsc/osmo_bsc_lcls.c@243
PS1, Line 243: 		get_value_string(bsc_lcls_mode_names, conn->sccp.msc->lcls_mode));
this and the others below get shorter with bsc_lcls_mode_name().


https://gerrit.osmocom.org/#/c/11565/1/src/osmo-bsc/osmo_bsc_lcls.c@252
PS1, Line 252: 		struct mgcp_conn_peer mdcx_info = (struct mgcp_conn_peer){
> whitespace missing before {
This is a struct initializer, so I'd favor Max's formatting choice: not place a space. Compare:

  libosmocore/src/gsm/gsm0808_utils.c
  990:		*dst = (struct gsm0808_cell_id_list2){
  996:	*dst = (struct gsm0808_cell_id_list2){

and various others



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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie91cc70de20ade2bfa3a1a108c731341f5e739bb
Gerrit-Change-Number: 11565
Gerrit-PatchSet: 1
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-CC: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Comment-Date: Mon, 05 Nov 2018 03:02:37 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181105/ad23763b/attachment.html>


More information about the gerrit-log mailing list