Attention is currently required from: dexter, laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35152?usp=email )
Change subject: mgcp-client: Transmit remote IP addr in CRCX if known and port=0 ...................................................................... Patch Set 6: Code-Review+1 (3 comments) File src/libosmo-mgcp-client/mgcp_client.c: https://gerrit.osmocom.org/c/osmo-mgw/+/35152/comment/b0323a8d_2b31bbce PS3, Line 1323: urn - > I'm not following you here. […] I mean: not return -EINVAL, instead skip the PRINTF("c=IN"). >From below comment about your preference, i gather that you don't agree with >this point. I think you're wrong, but i'm fine to live with your choice so one >day i can say "i told you so" =) anyway, here are my reasons, if you'd like to know. there are different causes for not having a valid address. this patch handles the causes differently: - when the flag is not set, omit the addr and send the rest. - when the flag is set, but the addr is empty, fail the entire message, do not send anything. i think it would be less complexity and more stable to just always omit and send the rest, no matter what the cause is for not having a valid IP address. that way we don't obscurely fail sending a message at all if some flag combination is set weirdly. Why do those flags even exist, if the value itself can indicate presence or not... https://gerrit.osmocom.org/c/osmo-mgw/+/35152/comment/b1c0ef6e_008b4619 PS3, Line 1339: et > I prefer the way it is now. […] Done https://gerrit.osmocom.org/c/osmo-mgw/+/35152/comment/dd85524f_d2d4fc6c PS3, Line 1353: codecs > Whichever gets first into master makes the other one rebase it, I don't > really care and it's fine ea […] ok, the other patch isn't ready yet until it has backwards compat, so go ahead. would be good if a C test covers this, to make sure it stays intact. -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35152?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: I30165dbac5e484011d0acf46af36f105954a501d Gerrit-Change-Number: 35152 Gerrit-PatchSet: 6 Gerrit-Owner: pespin <pes...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pma...@sysmocom.de> Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Reviewer: neels <nhofm...@sysmocom.de> Gerrit-Attention: laforge <lafo...@osmocom.org> Gerrit-Attention: pespin <pes...@sysmocom.de> Gerrit-Attention: dexter <pma...@sysmocom.de> Gerrit-Comment-Date: Wed, 06 Dec 2023 01:26:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: neels <nhofm...@sysmocom.de> Comment-In-Reply-To: pespin <pes...@sysmocom.de> Gerrit-MessageType: comment
