[asterisk-commits] rizzo: branch rizzo/astobj2 r47373 - /team/rizzo/astobj2/channels/chan_sip.c

asterisk-commits at lists.digium.com asterisk-commits at lists.digium.com
Thu Nov 9 09:19:05 MST 2006


Author: rizzo
Date: Thu Nov  9 10:19:05 2006
New Revision: 47373

URL: http://svn.digium.com/view/asterisk?view=rev&rev=47373
Log:
rename the "owner" field in a sip_pkt to "pvt", as it
is too confusing otherwise ("owner" is used to indicate a
channel in a nearby struct). Most of the usages are in a
single function.

Trunk candidate, definitely.

On passing note that __sip_ack(), when called by handle_response()
with resp == 491, does not free the packet, but because
the packet is not linked in the list anymore, this possibly
causes a memory leak. If not, it is a very obscure thing.

Need to consider this as a possible bug in trunk.


Modified:
    team/rizzo/astobj2/channels/chan_sip.c

Modified: team/rizzo/astobj2/channels/chan_sip.c
URL: http://svn.digium.com/view/asterisk/team/rizzo/astobj2/channels/chan_sip.c?view=diff&rev=47373&r1=47372&r2=47373
==============================================================================
--- team/rizzo/astobj2/channels/chan_sip.c (original)
+++ team/rizzo/astobj2/channels/chan_sip.c Thu Nov  9 10:19:05 2006
@@ -1056,7 +1056,7 @@
 	int method;				/*!< SIP method for this packet */
 	int seqno;				/*!< Sequence number */
 	unsigned int flags;			/*!< non-zero if this is a response packet (e.g. 200 OK) */
-	struct sip_pvt *owner;			/*!< Owner AST call */
+	struct sip_pvt *pvt;			/*!< Owner AST call */
 	int retransid;				/*!< Retransmission ID */
 	int timer_a;				/*!< SIP timer A, retransmission timer */
 	int timer_t1;				/*!< SIP Timer T1, estimated RTT or 500 ms */
@@ -1945,9 +1945,10 @@
 {
 	struct sip_pkt *pkt = data, *prev, *cur = NULL;
 	int reschedule = DEFAULT_RETRANS;
+	struct sip_pvt *pvt = pkt->pvt;	/* XXX we assume it is not null. maybe should check ? */
 
 	/* Lock channel PVT */
-	sip_pvt_lock(pkt->owner);
+	sip_pvt_lock(pvt);
 
 	if (pkt->retrans < MAX_RETRANS) {
 		pkt->retrans++;
@@ -1975,48 +1976,49 @@
  				ast_log(LOG_DEBUG, "** SIP timers: Rescheduling retransmission %d to %d ms (t1 %d ms (Retrans id #%d)) \n", pkt->retrans +1, siptimer_a, pkt->timer_t1, pkt->retransid);
  		} 
 
-		if (sip_debug_test_pvt(pkt->owner)) {
-			const struct sockaddr_in *dst = sip_real_dst(pkt->owner);
+		if (sip_debug_test_pvt(pkt->pvt)) {
+			const struct sockaddr_in *dst = sip_real_dst(pvt);
 			ast_verbose("Retransmitting #%d (%s) to %s:%d:\n%s\n---\n",
-				pkt->retrans, sip_nat_mode(pkt->owner),
+				pkt->retrans, sip_nat_mode(pvt),
 				ast_inet_ntoa(dst->sin_addr),
 				ntohs(dst->sin_port), pkt->data);
 		}
 
-		append_history(pkt->owner, "ReTx", "%d %s", reschedule, pkt->data);
-		__sip_xmit(pkt->owner, pkt->data, pkt->packetlen);
-		sip_pvt_unlock(pkt->owner);
+		append_history(pvt, "ReTx", "%d %s", reschedule, pkt->data);
+		__sip_xmit(pvt, pkt->data, pkt->packetlen);
+		sip_pvt_unlock(pvt);
 		return  reschedule;
 	} 
 	/* Too many retries */
-	if (pkt->owner && pkt->method != SIP_OPTIONS) {
+	if (pkt->method != SIP_OPTIONS) {
 		if (ast_test_flag(pkt, FLAG_FATAL) || sipdebug)	/* Tell us if it's critical or if we're debugging */
-			ast_log(LOG_WARNING, "Maximum retries exceeded on transmission %s for seqno %d (%s %s)\n", pkt->owner->callid, pkt->seqno, (ast_test_flag(pkt, FLAG_FATAL)) ? "Critical" : "Non-critical", (ast_test_flag(pkt, FLAG_RESPONSE)) ? "Response" : "Request");
+			ast_log(LOG_WARNING, "Maximum retries exceeded on transmission %s for seqno %d (%s %s)\n", pvt->callid, pkt->seqno, (ast_test_flag(pkt, FLAG_FATAL)) ? "Critical" : "Non-critical", (ast_test_flag(pkt, FLAG_RESPONSE)) ? "Response" : "Request");
 	} else {
 		if ((pkt->method == SIP_OPTIONS) && sipdebug)
-			ast_log(LOG_WARNING, "Cancelling retransmit of OPTIONs (call id %s) \n", pkt->owner->callid);
-	}
-	append_history(pkt->owner, "MaxRetries", "%s", (ast_test_flag(pkt, FLAG_FATAL)) ? "(Critical)" : "(Non-critical)");
+			ast_log(LOG_WARNING, "Cancelling retransmit of OPTIONs (call id %s) \n", pvt->callid);
+	}
+	append_history(pvt, "MaxRetries", "%s", (ast_test_flag(pkt, FLAG_FATAL)) ? "(Critical)" : "(Non-critical)");
  		
 	pkt->retransid = -1;
 
 	if (ast_test_flag(pkt, FLAG_FATAL)) {
 		/* the next call is unsafe, because we are called without dialoglock held,
-		 * and pkt->owner could disappear while we unlock it
+		 * and pvt could disappear while we unlock it
 		 */
-		lock_pvt_and_owner(pkt->owner, 0 /* try forever */);
-		if (pkt->owner->owner) {
-			ast_set_flag(&pkt->owner->flags[0], SIP_ALREADYGONE);
-			ast_log(LOG_WARNING, "Hanging up call %s - no reply to our critical packet.\n", pkt->owner->callid);
-			ast_queue_hangup(pkt->owner->owner);
-			ast_channel_unlock(pkt->owner->owner);
+		lock_pvt_and_owner(pvt, 0 /* try forever */);
+		if (pvt->owner) {
+			ast_set_flag(&pvt->flags[0], SIP_ALREADYGONE);
+			ast_log(LOG_WARNING, "Hanging up call %s - no reply to our critical packet.\n", pvt->callid);
+			ast_queue_hangup(pvt->owner);
+			ast_channel_unlock(pvt->owner);
 		} else {
 			/* If no channel owner, destroy now */
-			ast_set_flag(&pkt->owner->flags[0], SIP_NEEDDESTROY);	
-		}
-	}
+			ast_set_flag(&pvt->flags[0], SIP_NEEDDESTROY);	
+		}
+	}
+	/* XXX this code needs to be fixed */
 	/* In any case, go ahead and remove the packet */
-	for (prev = NULL, cur = pkt->owner->packets; cur; prev = cur, cur = cur->next) {
+	for (prev = NULL, cur = pvt->packets; cur; prev = cur, cur = cur->next) {
 		if (cur == pkt)
 			break;
 	}
@@ -2024,14 +2026,15 @@
 		if (prev)
 			prev->next = cur->next;
 		else
-			pkt->owner->packets = cur->next;
-		sip_pvt_unlock(pkt->owner);
+			pvt->packets = cur->next;
+		pkt->pvt = pvt_unref(pvt);	/* release the pvt */
+		sip_pvt_unlock(pvt);
 		free(cur);
 		pkt = NULL;
 	} else
 		ast_log(LOG_WARNING, "Weird, couldn't find packet owner!\n");
 	if (pkt)
-		sip_pvt_unlock(pkt->owner);
+		sip_pvt_unlock(pvt);
 	return 0;
 }
 
@@ -2049,7 +2052,7 @@
 	pkt->method = sipmethod;
 	pkt->packetlen = len;
 	pkt->next = p->packets;
-	pkt->owner = p;
+	pkt->pvt = pvt_ref(p);
 	pkt->seqno = seqno;
 	if (resp)
 		ast_set_flag(pkt, FLAG_RESPONSE);
@@ -2067,7 +2070,7 @@
 	pkt->next = p->packets;
 	p->packets = pkt;
 
-	__sip_xmit(pkt->owner, pkt->data, pkt->packetlen);	/* Send packet */
+	__sip_xmit(pkt->pvt, pkt->data, pkt->packetlen);	/* Send packet */
 	if (sipmethod == SIP_INVITE) {
 		/* Note this is a pending invite */
 		p->pendinginvite = seqno;
@@ -2167,6 +2170,8 @@
 					ast_log(LOG_DEBUG, "** SIP TIMER: Cancelling retransmit of packet (reply received) Retransid #%d\n", cur->retransid);
 				ast_sched_del(sched, cur->retransid);
 			}
+			if (cur->pvt)	/* XXX should be always */
+				cur->pvt = pvt_unref(cur->pvt);
 			if (!reset)
 				free(cur);
 			break;
@@ -12388,7 +12393,7 @@
 	if ((resp >= 100) && (resp <= 199))
 		__sip_semi_ack(p, seqno, 0, sipmethod);
 	else
-		__sip_ack(p, seqno, 0, sipmethod, resp == 491 ? TRUE : FALSE);
+		__sip_ack(p, seqno, 0, sipmethod, resp == 491 ? TRUE : FALSE);	/* XXX why true on 491 ? */
 
 	/* Get their tag if we haven't already */
 	if (ast_strlen_zero(p->theirtag) || (resp >= 200)) {



More information about the asterisk-commits mailing list