This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
tnt gerrit-no-reply at lists.osmocom.orgtnt has posted comments on this change. ( https://gerrit.osmocom.org/13242 ) Change subject: libmsc: Allow different channel types to be requested as silent calls ...................................................................... Patch Set 1: (8 comments) Tx for the review, will fix that up. https://gerrit.osmocom.org/#/c/13242/1//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/#/c/13242/1//COMMIT_MSG@10 PS1, Line 10: Signed-off-by: Sylvain Munaut <tnt at 246tNt.com> > No signed-off-by in osmocom repos. Ack https://gerrit.osmocom.org/#/c/13242/1/src/libmsc/msc_vty.c File src/libmsc/msc_vty.c: https://gerrit.osmocom.org/#/c/13242/1/src/libmsc/msc_vty.c@1072 PS1, Line 1072: uint16_t port = 0; > no initialization needed here afaiu. Ack https://gerrit.osmocom.org/#/c/13242/1/src/libmsc/silent_call.c File src/libmsc/silent_call.c: https://gerrit.osmocom.org/#/c/13242/1/src/libmsc/silent_call.c@40 PS1, Line 40: char traffic_ip[INET_ADDRSTRLEN]; > worth using INET6_ADDRSTRLEN here in case we want to support ipv6? Not sure if it makes sense in thi […] Everywhere else in the code we use INET_ADDRSTRLEN. So I'd rather keep it consistent. If at some point we want ipv6 the grep will turn up this code and hopefully it will be reviewed for ipv6 compat along with the rest. Using INET6_ADDRSTRLEN I think might make it look like it was tested or is v6 compatible which ... it's probably not at all and would be untested because the rest isn't v6 compatible. https://gerrit.osmocom.org/#/c/13242/1/src/libmsc/silent_call.c@91 PS1, Line 91: osmo_timer_schedule(&scd->timer, 0, 0); > Why not calling timer_cb directly around line 120? You are immediatelly firing this cb, and only on […] Because at this point in the code, I'm in the callback of a state machine which isn't in the right state to accept ran_conn_communicating() ... it will only be in the right state when the rest of the processing of the callback list is done. And I can't change the order of the callbacks because ... well the entire rest of the code base expects stuff to go in that exact order. I tried like 4 different approaches to solve this and this is the lest ugly/most self contained approach. https://gerrit.osmocom.org/#/c/13242/1/src/libmsc/silent_call.c@99 PS1, Line 99: } rtp_addr; > Is this conversion from sockaddr_in to sockaddr_storage through union valid? Never seen it before. Why wouldn't it be ? sockaddr_storage is just 'something big enough'. unions members are guaranteed to start at the beginning ... so I don't see an issue here. https://gerrit.osmocom.org/#/c/13242/1/src/libmsc/silent_call.c@107 PS1, Line 107: gsm0808_speech_codec_from_chan_type(&scl.codec[i], scd->ct.perm_spch[i]); > Would it make sense to re-use enc_speech_codec_list() from a_iface. […] I'm not convinced ... it literally just saves 1 for loop, the bulk of the work is done by gsm0808_speech_codec_from_chan_type. If people insist I can do it, but I'm not even sure where to put/declare it ... I guess it would belong to be in libosmocore in gsm0808_utils thus introducing a cross repo version dependency, seems a bit much. https://gerrit.osmocom.org/#/c/13242/1/src/libmsc/silent_call.c@120 PS1, Line 120: break; > Would be clearer using a return 0 here, otherwise it's confusing scheduling scd timer then having a […] Ack https://gerrit.osmocom.org/#/c/13242/1/src/libmsc/silent_call.c@187 PS1, Line 187: struct gsm0808_channel_type *ct, > ct is not changed in this function - why not make it const similar to traffic_dst_ip? Ack -- To view, visit https://gerrit.osmocom.org/13242 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82645708dd27864cf33ea9cc993ead0983415602 Gerrit-Change-Number: 13242 Gerrit-PatchSet: 1 Gerrit-Owner: tnt <tnt at 246tNt.com> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: tnt <tnt at 246tNt.com> Gerrit-CC: Max <msuraev at sysmocom.de> Gerrit-CC: Pau Espin Pedrol <pespin at sysmocom.de> Gerrit-Comment-Date: Wed, 13 Mar 2019 20:13:51 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190313/da7bff12/attachment.htm>