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.