Attention is currently required from: dexter, laforge, neels, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/36361?usp=email )
Change subject: Change msgb ownership in processing of received msgb
......................................................................
Patch Set 2:
(4 comments)
File src/libosmo-mgcp/mgcp_network.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/36361/comment/054b7bba_98b3a44e
PS2, Line 1191: return rc
missing `msgb_free()`?
https://gerrit.osmocom.org/c/osmo-mgw/+/36361/comment/2e47f083_0389a5bc
PS2, Line 1205: return rc
missing `msgb_free()`?
https://gerrit.osmocom.org/c/osmo-mgw/+/36361/comment/949625fb_1bc94e6f
PS2, Line 1212: return rc
missing `msgb_free()`?
File src/libosmo-mgcp/mgcp_osmux.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/36361/comment/e31c09c9_cb3f3273
PS2, Line 217: return -1;
missing `msgb_free()` here and below? AFAIU, if a function claims to take ownership, then it's also responsible for `free()`ing it in all error paths? If not, this should be clarified in the docs above.
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/36361?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: I6a331f3c6b2eb51ea312ac6ef8c357185ddb79cf
Gerrit-Change-Number: 36361
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(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-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 20 Mar 2024 07:36:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/36363?usp=email )
Change subject: Convert RTP/RTCP/OSMUX I/O from osmo_fd to osmo_io
......................................................................
Patch Set 1:
(2 comments)
File src/libosmo-mgcp/mgcp_network.c:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-15252):
https://gerrit.osmocom.org/c/osmo-mgw/+/36363/comment/1d6b142a_74778328
PS1, Line 1051: LOGP(DRTP, LOGL_DEBUG, "sending %i bytes length packet to %s ...\n", msgb_length(msg),
Use %d instead of %i
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-15252):
https://gerrit.osmocom.org/c/osmo-mgw/+/36363/comment/9c7a5c75_fc5863d1
PS1, Line 1492: proto = (iofd == conn_src->end.rtp)? MGCP_PROTO_RTP : MGCP_PROTO_RTCP;
spaces required around that '?' (ctx:VxW)
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/36363?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: I8471960d5d8088a70cf105f2f40dfa5d5458169a
Gerrit-Change-Number: 36363
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-CC: Jenkins Builder
Gerrit-Comment-Date: Tue, 19 Mar 2024 20:23:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: laforge.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-mgw/+/36361?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified-1 by Jenkins Builder
Change subject: Change msgb ownership in processing of received msgb
......................................................................
Change msgb ownership in processing of received msgb
The old approach was: rtp_data_net() reads a msgb from the incomging
socket, calls through whatever function chain and in the end free's it.
So none of the intermediate functions was permitted to take msgb
ownership.
This was a good choice as all processing would happen synchronously,
up to the point where that msgb was written on the output RTP socket.
Let's change this from passing msgb ownership throug the whole call
chain, through rx_rtp() to the various *_dispatch_rtp() functions.
This is required for upcoming migration to osmo_io, as in that case the
write (sendto) calls are asynchronous and hence msgb ownership needs
to be transferred.
Change-Id: I6a331f3c6b2eb51ea312ac6ef8c357185ddb79cf
---
M TODO-RELEASE
M src/libosmo-mgcp/mgcp_e1.c
M src/libosmo-mgcp/mgcp_iuup.c
M src/libosmo-mgcp/mgcp_network.c
M src/libosmo-mgcp/mgcp_osmux.c
5 files changed, 130 insertions(+), 40 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/61/36361/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/36361?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: I6a331f3c6b2eb51ea312ac6ef8c357185ddb79cf
Gerrit-Change-Number: 36361
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset