Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kgenjac/save sip reason state on callback #3210

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 21 additions & 20 deletions src/plugins/janus_sip.c
Original file line number Diff line number Diff line change
Expand Up @@ -1504,6 +1504,7 @@ static void janus_sip_media_reset(janus_sip_session *session) {
gpointer janus_sip_sofia_thread(gpointer user_data);
/* Sofia callbacks */
void janus_sip_sofia_callback(nua_event_t event, int status, char const *phrase, nua_t *nua, nua_magic_t *magic, nua_handle_t *nh, nua_hmagic_t *hmagic, sip_t const *sip, tagi_t tags[]);
void janus_sip_save_reason(sip_t const *sip, janus_sip_session *session);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea about the helper method! This should probably be static, but I see other functions aren't either, so that's something I can fix myself in a later commit.

/* SDP parsing and manipulation */
void janus_sip_sdp_process(janus_sip_session *session, janus_sdp *sdp, gboolean answer, gboolean update, gboolean *changed);
char *janus_sip_sdp_manipulate(janus_sip_session *session, janus_sdp *sdp, gboolean answer);
Expand Down Expand Up @@ -5121,30 +5122,12 @@ void janus_sip_sofia_callback(nua_event_t event, int status, char const *phrase,
break;
case nua_i_bye: {
JANUS_LOG(LOG_VERB, "[%s][%s]: %d %s\n", session->account.username, nua_event_name(event), status, phrase ? phrase : "??");
if(sip->sip_reason && sip->sip_reason->re_text) {
session->hangup_reason_header = g_strdup(sip->sip_reason->re_text);
janus_sip_remove_quotes(session->hangup_reason_header);
}
if(sip->sip_reason && sip->sip_reason->re_protocol) {
session->hangup_reason_header_protocol = g_strdup(sip->sip_reason->re_protocol);
}
if(sip->sip_reason && sip->sip_reason->re_cause) {
session->hangup_reason_header_cause = g_strdup(sip->sip_reason->re_cause);
}
janus_sip_save_reason(sip, session);
break;
}
case nua_i_cancel: {
JANUS_LOG(LOG_VERB, "[%s][%s]: %d %s\n", session->account.username, nua_event_name(event), status, phrase ? phrase : "??");
if(sip->sip_reason && sip->sip_reason->re_text) {
session->hangup_reason_header = g_strdup(sip->sip_reason->re_text);
janus_sip_remove_quotes(session->hangup_reason_header);
}
if(sip->sip_reason && sip->sip_reason->re_protocol) {
session->hangup_reason_header_protocol = g_strdup(sip->sip_reason->re_protocol);
}
if(sip->sip_reason && sip->sip_reason->re_cause) {
session->hangup_reason_header_cause = g_strdup(sip->sip_reason->re_cause);
}
janus_sip_save_reason(sip, session);
break;
}
case nua_i_invite: {
Expand Down Expand Up @@ -5788,6 +5771,7 @@ void janus_sip_sofia_callback(nua_event_t event, int status, char const *phrase,
break;
}
} else if(status == 401 || status == 407) {
janus_sip_save_reason(sip, session);
const char *scheme = NULL;
const char *realm = NULL;
if(status == 401) {
Expand Down Expand Up @@ -5846,6 +5830,7 @@ void janus_sip_sofia_callback(nua_event_t event, int status, char const *phrase,
} else if(status == 700) {
JANUS_LOG(LOG_VERB, "Handling SDP answer in ACK\n");
} else if(status >= 400 && status != 700) {
janus_sip_save_reason(sip, session);
break;
}
if(ssip == NULL) {
Expand Down Expand Up @@ -6300,6 +6285,22 @@ void janus_sip_sofia_callback(nua_event_t event, int status, char const *phrase,
}
}

void janus_sip_save_reason(sip_t const *sip, janus_sip_session *session) {
if (!sip || !session)
return;

if (sip->sip_reason && sip->sip_reason->re_text) {
session->hangup_reason_header = g_strdup(sip->sip_reason->re_text);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add a

g_free(session->hangup_reason_header);

before the g_strdup, so that we avoid memory leaks when a new reason replaces one received before. This was an issue we had already, apparently, so it may be a goot opportunity to fix this for good. The same should be done for the other two g_strdup before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah actually I see they're freed in nua_i_state so it may not be needed.

Copy link
Contributor Author

@kenangenjac kenangenjac May 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I saw that it is freed in nua_i_state so I didn't add it in the method.

janus_sip_remove_quotes(session->hangup_reason_header);
}
if (sip->sip_reason && sip->sip_reason->re_protocol) {
session->hangup_reason_header_protocol = g_strdup(sip->sip_reason->re_protocol);
}
if (sip->sip_reason && sip->sip_reason->re_cause) {
session->hangup_reason_header_cause = g_strdup(sip->sip_reason->re_cause);
}
}

void janus_sip_sdp_process(janus_sip_session *session, janus_sdp *sdp, gboolean answer, gboolean update, gboolean *changed) {
if(!session || !sdp)
return;
Expand Down