sipsess: add cancel reason to close handler #1325
Conversation
src/sip/strans.c
Outdated
| mem_deref(st->mb); | ||
| } | ||
|
|
||
| const struct sip_msg *sipsess_cancel_msg(struct sip_strans *st) { |
There was a problem hiding this comment.
if st is NULL please return NULL
| @@ -219,6 +223,7 @@ static bool cancel_handler(struct sip *sip, const struct sip_msg *msg) | |||
|
|
|||
| case TRYING: | |||
| case PROCEEDING: | |||
| st->cancel_msg = mem_ref((void *)msg); | |||
There was a problem hiding this comment.
please do mem_deref first to avoid memory leaks
There was a problem hiding this comment.
I don't quite get what this should achieve? I store a ref to this message, calling mem_deref first would effectively keep the ref counter the same right?
I tried to copy the code for strans.msg which does a mem_ref before storing it in the strans.
There was a problem hiding this comment.
if cancel_msg points to another object before calling, there will be a leak.
please look at this example:
src/http/client.c-1000- mem_deref(cli->tls);
src/http/client.c:1001: cli->tls = mem_ref(tls);
There was a problem hiding this comment.
Ah I see. Sorry for the confusion. I didn't assume that this codepath could be taken more than once, but better safe than sorry 👍
|
This doesn't work yet, the |
…psess->st gets cleared in sip_treply
src/sip/sip.h
Outdated
| @@ -104,3 +104,5 @@ int sip_keepalive_udp(struct sip_keepalive *ka, struct sip *sip, | |||
| /* sip_conncfg */ | |||
| struct sip_conncfg *sip_conncfg_find(struct sip *sip, | |||
| const struct sa *paddr); | |||
|
|
|||
| const struct sip_msg *sipsess_cancel_msg(struct sip_strans *st); | |||
There was a problem hiding this comment.
this line can be moved to re_sip.h
src/sip/strans.c
Outdated
| @@ -44,7 +45,6 @@ struct sip_strans { | |||
| bool invite; | |||
| }; | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
unrelated change, please revert ...
|
Thanks for the updates ... This looks good now ... |
* store cancel message in sip_trans and pass to termination/close handler * protect against null pointer access, and get cancel message before sipsess->st gets cleared in sip_treply * deref cancel message before setting it to avoid leaks and move function to re_sip.h * fix typo * Revert unintended deleted empty line
I want to display the provided reason in Cancel messages to the user of baresip, but as far as I could tell the Cancel messages are not communicated outside of libre.
Related to this feature request: baresip/baresip#3363
As per the feedback in #1309 I implemented passing the cancel message in a way that shouldn't break the API. I didn't want to force push over the other PR, so I made a new one.
The one thing I'm not happy with is that I needed to duplicate the forward definition of the function to get the message from the
sip_trans. If you have any suggestions to solve this more elegantly, I'd be happy to implement them.SIP trace for reference what I want to achieve. the important line here is this:
Reason: SIP;cause=200;text="Call completed elsewhere"