[Asterisk-code-review] chan_pjsip: Transfer failure sip result passed through channel: Trans... (asterisk[16])

[Dan Cropp]

Dan Cropp has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/15368 )

Change subject: chan_pjsip: Transfer failure sip result passed through channel: Transfer parameter returns the sip protocol error code control: Redirect support additional transfer protocol parameter app_transfer: TRANSFERSTATUSPROTOCOL channel variable addition
......................................................................


Patch Set 1:

> Patch Set 1:
> 
> > Patch Set 1:
> > 
> > > Patch Set 1:
> > > 
> > > Just a heads up (I haven't reviewed this fully) but this is not suitable for inclusion into 16 or 18 as-is. This changes the ast_transfer API in a breaking fashion.
> > 
> > Thank you Joshua.
> > 
> > I believe I modified everywhere ast_transfer was called to use the additional parameter.  Is it unacceptable to do this?
> > 
> > I believe I could make it work without the additional parameter.  It would require setting the return value to the error. Then, in code that calls ast_transfer, it would need to process the result (checking for anything 300+).  Basically, setting it's process error value and change the original value to AST_SUCCESS/AST_FAILURE.  Would this be a more acceptable approach (provided it's well documented)?
> 
> We don't change APIs in a manner that breaks ABI compatibility in release branches, unless absolutely necessary to fix something like a security issue or other serious issue.
> 
> I don't think changing the return value is acceptable, as while ABI compatible that's still a change to the API and a behavior change.
> 
> A second API call being added is perfectly fine, however, as long as the existing one remains and continues to function.

Thank you.

So I could leave the existing alone (which would allow me to not touch res/stasis/control.c).  Make a new ast_transfer_protocol for my change and it would be sufficient for 16 and 18?


-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15368
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Ia6a94784b4925628af122409cdd733c9f29abfc4
Gerrit-Change-Number: 15368
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Cropp <dan at amtelco.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-CC: Joshua Colp <jcolp at sangoma.com>
Gerrit-Comment-Date: Fri, 22 Jan 2021 15:48:08 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210122/f1d038ee/attachment-0001.html>