December 2023 - gerrit-log - lists.osmocom.org

[S] Change in osmo-mgw[master]: mgcp_client_fsm: allocate struct mgcp_conn_peer dynamically

by neels

Attention is currently required from: dexter, laforge. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34404?usp=email ) Change subject: mgcp_client_fsm: allocate struct mgcp_conn_peer dynamically ...................................................................... Patch Set 13: (1 comment) File src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34404/comment/f7860876_b8a785a5 PS12, Line 698: *ci->verb_info > This patch is obviously untested, because this here cannot work. […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34404?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I523d0fcb020f7d46323c497a4be9ee00d5f242ba Gerrit-Change-Number: 34404 Gerrit-PatchSet: 13 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Fri, 08 Dec 2023 06:07:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 6 months

1
0
0 0

[S] Change in osmo-mgw[master]: fix possible NULL deref on early media

by neels

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35263?usp=email ) Change subject: fix possible NULL deref on early media ...................................................................... fix possible NULL deref on early media end.codec may be NULL, depending on whether a single matching codec could be found or not, i.e. based on external input. So let's not crash if an RTP packet arrives while end.codec == NULL. Change-Id: I9bfb55a343b3f2b1459e0aba4ee71a6133b992b3 --- M src/libosmo-mgcp/mgcp_network.c 1 file changed, 16 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/63/35263/1 diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index 674c096..e37437d 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -1514,8 +1514,9 @@ return -1; /* Handle AMR frame format conversion (octet-aligned vs. bandwith-efficient) */ - if (mc->proto == MGCP_PROTO_RTP && - mgcp_codec_amr_align_mode_is_indicated(conn_src->end.codec)) { + if (mc->proto == MGCP_PROTO_RTP + && conn_src->end.codec + && mgcp_codec_amr_align_mode_is_indicated(conn_src->end.codec)) { /* Make sure that the incoming AMR frame format matches the frame format that the call agent has * communicated via SDP when the connection was created/modfied. */ int oa = amr_oa_check((char*)msgb_data(msg), msgb_length(msg)); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35263?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I9bfb55a343b3f2b1459e0aba4ee71a6133b992b3 Gerrit-Change-Number: 35263 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 6 months

1
0
0 0

[S] Change in osmo-mgw[master]: mgcp_client_fsm: explain member param in struct mgcp_conn_peer better

by neels

Attention is currently required from: dexter. neels has uploaded a new patch set (#12) to the change originally created by dexter. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34350?usp=email ) The following approvals got outdated and were removed: Verified+1 by Jenkins Builder The change is no longer submittable: Verified is unsatisfied now. Change subject: mgcp_client_fsm: explain member param in struct mgcp_conn_peer better ...................................................................... mgcp_client_fsm: explain member param in struct mgcp_conn_peer better The struct member param specifies additional codec parameters. Let's improve its explaination. Change-Id: Iea4dc1e72fccaa464ce503fae88b5d8a867b1d19 Related: OS#6171 --- M include/osmocom/mgcp_client/mgcp_client_fsm.h 1 file changed, 15 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/50/34350/12 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34350?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Iea4dc1e72fccaa464ce503fae88b5d8a867b1d19 Gerrit-Change-Number: 34350 Gerrit-PatchSet: 12 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 year, 6 months

1
0
0 0

[S] Change in osmo-mgw[master]: mgcp_client_fsm: allocate struct mgcp_conn_peer dynamically

by neels

Attention is currently required from: dexter, laforge, neels. neels has uploaded a new patch set (#13) to the change originally created by dexter. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34404?usp=email ) The following approvals got outdated and were removed: Code-Review+1 by laforge, Code-Review-1 by neels, Verified+1 by Jenkins Builder Change subject: mgcp_client_fsm: allocate struct mgcp_conn_peer dynamically ...................................................................... mgcp_client_fsm: allocate struct mgcp_conn_peer dynamically The struct mgcp_conn_peer is allocated statically at the moment. This is an ABI compatibility in case the struct changes. Add an alloc function for this struct that API users can use, and indicate its use in a comment on struct mgcp_conn_peer. It is not necessary to dynamically allocate the mgcp_conn_peer struct within libosmo-mgcp-client, because the problem arises only when ABIs interact. Hence leaving mgcp_client_endpoint_fsm.c's use static. Related: OS#6171 Change-Id: I523d0fcb020f7d46323c497a4be9ee00d5f242ba --- M include/osmocom/mgcp_client/mgcp_client_fsm.h M src/libosmo-mgcp-client/mgcp_client_fsm.c 2 files changed, 38 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/04/34404/13 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34404?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I523d0fcb020f7d46323c497a4be9ee00d5f242ba Gerrit-Change-Number: 34404 Gerrit-PatchSet: 13 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 year, 6 months

1
0
0 0

[M] Change in osmo-mgw[master]: client: move some items to internal header

by neels

Attention is currently required from: laforge, pespin. Hello Jenkins Builder, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/35246?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review+1 by laforge, Code-Review+1 by pespin, Verified+1 by Jenkins Builder Change subject: client: move some items to internal header ...................................................................... client: move some items to internal header There is an upcoming modification of structs mgcp_msg and mgcp_response. When it is public API, it needs to be kept backwards compatible. But no libosmo-mgcp-client caller (in the osmocom-cni world) has used these structs in years and years. Everyone should use the higher level osmo_mgcpc_* API only. Move this legacy API to a private header, so we no longer need to worry about compatibility there. Related: OS#6171 Related: I798e02c6663376d3d52f4a74fc4b32411ce95bed Change-Id: I6d8bdda5c2ffa236e94a0b6111de4dbfac923187 --- M TODO-RELEASE M include/osmocom/mgcp_client/mgcp_client.h M include/osmocom/mgcp_client/mgcp_client_internal.h M src/libosmo-mgcp-client/mgcp_client_fsm.c 4 files changed, 99 insertions(+), 65 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/46/35246/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35246?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I6d8bdda5c2ffa236e94a0b6111de4dbfac923187 Gerrit-Change-Number: 35246 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 year, 6 months

1
0
0 0

[L] Change in osmo-mgw[master]: add fmtp string to ptmap: allow all possible fmtp

by neels

Attention is currently required from: laforge, neels, pespin. Hello Jenkins Builder, laforge, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email to look at the new patch set (#9). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: add fmtp string to ptmap: allow all possible fmtp ...................................................................... add fmtp string to ptmap: allow all possible fmtp Remove the limit of having only one AMR octet-aligned fmtp parameter per MGCP message. Instead allow any arbitrary fmtp options, one per every codec. Deprecate all use of struct mgcp_codec_param. Instead, store and pass plain fmtp strings. We need to know fmtp details only for AMR, and only to probe whether it is octet-aligned. So add a separate fmtp string parser that returns that information flexibly, as in if (osmo_mgcp_fmtp_get_int("octet-aligned", 0) == 1) ... Provide legacy shims that still act correctly for any callers that may pass the old struct mgcp_codec_param. (I'm not sure if we need to keep this, but we can always drop it in another patch.) Adjust one mgcp_test.c: instead of returning only the octet-aligned parameter, now osmo-mgw keeps and returns all the fmtp parameters that the user provided. So add the missing "mode-change-capability". Related: OS#6171 Change-Id: If58590bda8627519ff07e0b6f43aa47a274f052b --- M include/osmocom/mgcp/Makefile.am A include/osmocom/mgcp/fmtp.h M include/osmocom/mgcp/mgcp_codec.h M include/osmocom/mgcp/mgcp_common.h M include/osmocom/mgcp/mgcp_network.h M include/osmocom/mgcp_client/mgcp_client.h M include/osmocom/mgcp_client/mgcp_client_fsm.h M src/libosmo-mgcp-client/mgcp_client.c M src/libosmo-mgcp/Makefile.am A src/libosmo-mgcp/fmtp.c M src/libosmo-mgcp/mgcp_codec.c M src/libosmo-mgcp/mgcp_network.c M src/libosmo-mgcp/mgcp_protocol.c M src/libosmo-mgcp/mgcp_sdp.c M tests/mgcp/mgcp_test.c 15 files changed, 265 insertions(+), 100 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/00/34900/9 -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34900?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: If58590bda8627519ff07e0b6f43aa47a274f052b Gerrit-Change-Number: 34900 Gerrit-PatchSet: 9 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: dexter <pmaier(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 year, 6 months

1
0
0 0

[M] Change in osmo-mgw[master]: client: move some items to internal header

by neels

Attention is currently required from: laforge. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35246?usp=email ) Change subject: client: move some items to internal header ...................................................................... Patch Set 2: (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-mgw/+/35246/comment/95d25dcc_45d7d400 PS2, Line 12: mgcp > libosmo-mgcp-client Done -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35246?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I6d8bdda5c2ffa236e94a0b6111de4dbfac923187 Gerrit-Change-Number: 35246 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Fri, 08 Dec 2023 06:04:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Gerrit-MessageType: comment

1 year, 6 months

1
0
0 0

[S] Change in osmo-mgw[master]: mgcp_client_test: fix function name

by neels

neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35245?usp=email ) Change subject: mgcp_client_test: fix function name ...................................................................... mgcp_client_test: fix function name Change-Id: I213f9b2713ff80a16603b8342c204cb546cc782d --- M tests/mgcp_client/mgcp_client_test.c 1 file changed, 11 insertions(+), 2 deletions(-) Approvals: pespin: Looks good to me, approved Jenkins Builder: Verified diff --git a/tests/mgcp_client/mgcp_client_test.c b/tests/mgcp_client/mgcp_client_test.c index 37c5f6c..a39f19b 100644 --- a/tests/mgcp_client/mgcp_client_test.c +++ b/tests/mgcp_client/mgcp_client_test.c @@ -531,7 +531,7 @@ OSMO_ASSERT(!failures); } -static void test_map_pt_to_codec(void) +static void test_map_str_to_codec(void) { /* Full form */ OSMO_ASSERT(map_str_to_codec("PCMU/8000/1") == CODEC_PCMU_8000_1); @@ -703,7 +703,7 @@ test_mgcp_client_cancel(); test_sdp_section_start(); test_map_codec_to_pt_and_map_pt_to_codec(); - test_map_pt_to_codec(); + test_map_str_to_codec(); test_mgcp_client_e1_epname(); printf("Done\n"); -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35245?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I213f9b2713ff80a16603b8342c204cb546cc782d Gerrit-Change-Number: 35245 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 year, 6 months

1
0
0 0

[M] Change in osmo-mgw[master]: mgcp_client_fsm: allocate struct mgcp_conn_peer dynamically

by neels

Attention is currently required from: dexter. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34404?usp=email ) Change subject: mgcp_client_fsm: allocate struct mgcp_conn_peer dynamically ...................................................................... Patch Set 12: Code-Review-1 (1 comment) File src/libosmo-mgcp-client/mgcp_client_endpoint_fsm.c: https://gerrit.osmocom.org/c/osmo-mgw/+/34404/comment/5fbc5cee_2ebc817a PS12, Line 698: *ci->verb_info This patch is obviously untested, because this here cannot work. Above, '*ci = cleared_ci;' leaves ci->verb_info == NULL. Now we write data to that NULL pointer. This code made sense when ci->verb_info was a member of ci, not so when it is a pointer. But no need to fix this code, refreshing my earlier comment: The ABI problem arises when users assume a smaller size of the struct than libosmo-mgcp-client. So users of libosmo-mgcp-client should do dynamic allocation to ensure being ABI compatible; no need to change the libosmo-mgcp-client code itself, it is always ABI compatible with itself. So adding the API doc and allocation function is all that is needed in this patch. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34404?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: I523d0fcb020f7d46323c497a4be9ee00d5f242ba Gerrit-Change-Number: 34404 Gerrit-PatchSet: 12 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Fri, 08 Dec 2023 02:25:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 6 months

1
0
0 0

[S] Change in libosmocore[master]: tests/coding: fix -Wmaybe-uninitialized in test_pdtch()

by fixeria

fixeria has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35259?usp=email ) Change subject: tests/coding: fix -Wmaybe-uninitialized in test_pdtch() ...................................................................... tests/coding: fix -Wmaybe-uninitialized in test_pdtch() I am seeing this when building with gcc v13.2.1: tests/coding/coding_test.c: In function ‘test_pdtch’: tests/coding/coding_test.c:444:23: warning: ‘*result[<unknown>]’ may be used uninitialized 444 | result[len - 1] &= 0x7f; | ~~~~~~^~~~~~~~~ tests/coding/coding_test.c:448:23: warning: ‘*result[39]’ may be used uninitialized 448 | result[len - 1] &= 0x07; | ~~~~~~^~~~~~~~~ The idea here is to pre-clear some bits in the resulting buffer, because they're not going to be set during decoding of the burst bits. The problem is that result[] holds uninitialized data, so we're basically taking a 'garbage' octet and clear some of its bits. The remaining 'garbage' bits of that octet are overwritten by the decoder, so in the end we still get deterministic results. Let's make GCC happy by clearing all bits in the last octet. Change-Id: I24d79de8b3a5f4184b71414504657e5857498e0e --- M tests/coding/coding_test.c 1 file changed, 32 insertions(+), 2 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified diff --git a/tests/coding/coding_test.c b/tests/coding/coding_test.c index c9508f7..d536e44 100644 --- a/tests/coding/coding_test.c +++ b/tests/coding/coding_test.c @@ -441,11 +441,11 @@ case 34: case 54: l2[len - 1] &= 0x7f; - result[len - 1] &= 0x7f; + result[len - 1] = 0x00; break; case 40: l2[len - 1] &= 0x07; - result[len - 1] &= 0x07; + result[len - 1] = 0x00; break; } -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35259?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I24d79de8b3a5f4184b71414504657e5857498e0e Gerrit-Change-Number: 35259 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 year, 6 months

1
0
0 0

2025

2024

2023

2022

gerrit-log December 2023