From ef45b7b804a3ba79880afd218f6b3f3962c85ed7 Mon Sep 17 00:00:00 2001 From: Daniel-Constantin Mierla Date: Fri, 5 Jan 2024 11:55:03 +0100 Subject: [PATCH] tm: free new uac dlg in case of errors --- src/modules/tm/dlg.c | 21 +++++++++++++++------ src/modules/tm/rpc_uac.c | 1 - src/modules/tm/t_funcs.c | 2 +- src/modules/tm/t_lookup.c | 2 +- src/modules/tm/tm.c | 4 ++-- src/modules/tm/uac.c | 3 +-- 6 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/modules/tm/dlg.c b/src/modules/tm/dlg.c index 96483259c8c..8120a061ead 100644 --- a/src/modules/tm/dlg.c +++ b/src/modules/tm/dlg.c @@ -333,17 +333,25 @@ int new_dlg_uac(str *_cid, str *_ltag, unsigned int _lseq, str *_luri, memset(res, 0, sizeof(dlg_t)); /* Make a copy of Call-ID */ - if(str_duplicate(&res->id.call_id, _cid) < 0) + if(str_duplicate(&res->id.call_id, _cid) < 0) { + free_dlg(res); return -3; + } /* Make a copy of local tag (usually From tag) */ - if(str_duplicate(&res->id.loc_tag, _ltag) < 0) + if(str_duplicate(&res->id.loc_tag, _ltag) < 0) { + free_dlg(res); return -4; + } /* Make a copy of local URI (usually From) */ - if(str_duplicate(&res->loc_uri, _luri) < 0) + if(str_duplicate(&res->loc_uri, _luri) < 0) { + free_dlg(res); return -5; + } /* Make a copy of remote URI (usually To) */ - if(str_duplicate(&res->rem_uri, _ruri) < 0) + if(str_duplicate(&res->rem_uri, _ruri) < 0) { + free_dlg(res); return -6; + } /* Make a copy of local sequence (usually CSeq) */ res->loc_seq.value = _lseq; /* And mark it as set */ @@ -353,8 +361,9 @@ int new_dlg_uac(str *_cid, str *_ltag, unsigned int _lseq, str *_luri, if(calculate_hooks(*_d) < 0) { LM_ERR("error while calculating hooks\n"); - /* FIXME: free everything here */ - shm_free(res); + /* free everything here */ + free_dlg(res); + *_d = NULL; return -2; } #ifdef DIALOG_CALLBACKS diff --git a/src/modules/tm/rpc_uac.c b/src/modules/tm/rpc_uac.c index d461d123a13..e63da4fb835 100644 --- a/src/modules/tm/rpc_uac.c +++ b/src/modules/tm/rpc_uac.c @@ -387,7 +387,6 @@ static int get_hfblock(str *uri, struct hdr_field *hf, int proto, if(!append_str_list(sock_name->s, sock_name->len, &last, &total_len)) goto error; - /* inefficient - FIXME --andrei*/ if(!append_str_list(":", 1, &last, &total_len)) goto error; if(!append_str_list(portname->s, portname->len, &last, diff --git a/src/modules/tm/t_funcs.c b/src/modules/tm/t_funcs.c index d0975dab511..b9f9949d0fb 100644 --- a/src/modules/tm/t_funcs.c +++ b/src/modules/tm/t_funcs.c @@ -272,7 +272,7 @@ int t_relay_to( /* transaction previously found (E_SCRIPT) and msg==ACK => ack to neg. reply or ack to local trans. => process it and exit */ - /* FIXME: there's no way to distinguish here between acks to + /* - there's no way to distinguish here between acks to local trans. and neg. acks */ /* in normal operation we should never reach this point, if we do WARN(), it might hide some real bug (apart from possibly diff --git a/src/modules/tm/t_lookup.c b/src/modules/tm/t_lookup.c index 0ac9f6773d9..712f28fb7d5 100644 --- a/src/modules/tm/t_lookup.c +++ b/src/modules/tm/t_lookup.c @@ -429,7 +429,7 @@ static int matching_3261(struct sip_msg *p_msg, struct cell **trans, * which is interested in it (E2EACK* callbacks) * if ret==3 => partial match => we should at least * make sure the ACK is not for a negative reply - * (FIXME: ret==3 should never happen, it's a bug catch + * (note: ret==3 should never happen, it's a bug catch * case)*/ if(unlikely(ret == 1)) goto found; diff --git a/src/modules/tm/tm.c b/src/modules/tm/tm.c index b2866a685cc..0be958a0848 100644 --- a/src/modules/tm/tm.c +++ b/src/modules/tm/tm.c @@ -663,7 +663,7 @@ static int fixup_hostport2proxy(void **param, int param_no) } s.s = host; s.len = strlen(host); - proxy = mk_proxy(&s, port, 0); /* FIXME: udp or tcp? */ + proxy = mk_proxy(&s, port, 0); if(proxy == 0) { LM_ERR("bad host name in URI <%s>\n", host); return E_BAD_ADDRESS; @@ -2572,7 +2572,7 @@ int t_check_trans(struct sip_msg *msg) if(msg->REQ_METHOD == METHOD_ACK) { /* ack to neg. reply or ack to local trans. * => process it and end the script */ - /* FIXME: there's no way to distinguish here + /* - there's no way to distinguish here * between acks to local trans. and neg. acks */ if(unlikely(has_tran_tmcbs(t, TMCB_ACK_NEG_IN))) run_trans_callbacks( diff --git a/src/modules/tm/uac.c b/src/modules/tm/uac.c index f1c2b9d1706..206f69ef59d 100644 --- a/src/modules/tm/uac.c +++ b/src/modules/tm/uac.c @@ -936,8 +936,7 @@ int ack_local_uac(struct cell *trans, str *hdrs, str *body) ret = 0; fin: - /* TODO: ugly! */ - /* FIXME: the T had been obtain by t_lookup_ident()'ing for it, so, it is + /* note: the T had been obtain by t_lookup_ident()'ing for it, so, it is * ref-counted. The t_unref() can not be used, as it requests a valid SIP * message (all available might be the reply, but if AS goes wrong and * tries to ACK before the final reply is received, we still have to