[Asterisk-code-review] res pjsip/config transport: Allow reloading transports. (asterisk[13])

[Mark Michelson]

Mark Michelson has posted comments on this change.

Change subject: res_pjsip/config_transport: Allow reloading transports.
......................................................................


Patch Set 8:

(1 comment)

I'm curious about the extent to which testing was performed on this.

* When using UDP, and you alter the transport part-way through a call, I would suspect that a BYE sent from a remote device would not know to send to Asterisk's newly configured address/port.

* When using TCP, I would suspect that shutting down the transport while a call is up would cause the TCP connection to drop, which would result in a different sort of badness.

The same concerns go for other dialog-based SIP usage, like subscriptions. If I change the UDP transport on a subscription mid-dialog, then how will the remote device know where to send resubscription requests?

https://gerrit.asterisk.org/#/c/2125/8/res/res_pjsip/config_transport.c
File res/res_pjsip/config_transport.c:

Line 351: static int has_state_changed(struct ast_sip_transport_state *a, struct ast_sip_transport_state *b)
Why does this return different values in different places? Since the only use of this seems to be boolean-esque, it makes more sense to me for this to return either 0 or 1. If you want diagnostics about what specifically changed, then having more return values makes sense, but it would make more sense to return an enum instead of magic numbers.


-- 
To view, visit https://gerrit.asterisk.org/2125
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5e759850e25958117d4c02f62ceb7244d7ec9edf
Gerrit-PatchSet: 8
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-HasComments: Yes