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

[asterisk-commits at lists.digium.com]

Author: rizzo
Date: Thu Nov  9 09:56:45 2006
New Revision: 47368

URL: http://svn.digium.com/view/asterisk?view=rev&rev=47368
Log:
document usage of tech_pvt related to refcount handling.


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=47368&r1=47367&r2=47368
==============================================================================
--- team/rizzo/astobj2/channels/chan_sip.c (original)
+++ team/rizzo/astobj2/channels/chan_sip.c Thu Nov  9 09:56:45 2006
@@ -1599,22 +1599,22 @@
 	.description = "Session Initiation Protocol (SIP)",
 	.capabilities = ((AST_FORMAT_MAX_AUDIO << 1) - 1),
 	.properties = AST_CHAN_TP_WANTSJITTER | AST_CHAN_TP_CREATESJITTER,
-	.requester = sip_request_call,
-	.devicestate = sip_devicestate,
-	.call = sip_call,
-	.hangup = sip_hangup,
-	.answer = sip_answer,
-	.read = sip_read,
-	.write = sip_write,
-	.write_video = sip_write,
-	.indicate = sip_indicate,
-	.transfer = sip_transfer,
-	.fixup = sip_fixup,
-	.send_digit_begin = sip_senddigit_begin,
+	.requester = sip_request_call,	/* called with chan unlocked */
+	.devicestate = sip_devicestate,	/* called with chan unlocked (not chan-specific) */
+	.call = sip_call,		/* called with chan locked */
+	.hangup = sip_hangup,		/* called with chan locked */
+	.answer = sip_answer,		/* called with chan locked */
+	.read = sip_read,		/* called with chan locked */
+	.write = sip_write,		/* called with chan locked */
+	.write_video = sip_write,		/* called with chan locked */
+	.indicate = sip_indicate,	/* called with chan locked */
+	.transfer = sip_transfer,		/* called with chan locked */
+	.fixup = sip_fixup,		/* called with chan locked */
+	.send_digit_begin = sip_senddigit_begin,	/* called with chan unlocked */
 	.send_digit_end = sip_senddigit_end,
-	.bridge = ast_rtp_bridge,
+	.bridge = ast_rtp_bridge,	/* XXX chan unlocked ? */
 	.early_bridge = ast_rtp_early_bridge,
-	.send_text = sip_sendtext,
+	.send_text = sip_sendtext,	/* called with chan locked */
 };
 
 /**--- some list management macros. **/
@@ -2427,7 +2427,7 @@
 	Called from PBX core sendtext() application */
 static int sip_sendtext(struct ast_channel *ast, const char *text)
 {
-	struct sip_pvt *p = ast->tech_pvt;
+	struct sip_pvt *p = ast->tech_pvt;	/* chan is locked, no need to grab ref */
 	int debug = sip_debug_test_pvt(p);
 
 	if (debug)
@@ -2925,12 +2925,11 @@
 static int sip_call(struct ast_channel *ast, char *dest, int timeout)
 {
 	int res;
-	struct sip_pvt *p;
+	struct sip_pvt *p = ast->tech_pvt;	/* chan is locked, no need to grab reference */
 	struct varshead *headp;
 	struct ast_var_t *current;
 	const char *referer = NULL;   /* SIP refererer */	
 
-	p = ast->tech_pvt;
 	if ((ast->_state != AST_STATE_DOWN) && (ast->_state != AST_STATE_RESERVED)) {
 		ast_log(LOG_WARNING, "sip_call called on %s, neither down nor reserved\n", ast->name);
 		return -1;
@@ -3101,13 +3100,13 @@
 	}
 
 	/* Unlink us from the owner if we have one */
-	if (p->owner) {
+	if (p->owner) { /* XXX never taken ? */
+		ast_verbose("pvt_destructor %p owner->tech_pvt %p\n",
+			p, p->owner->tech_pvt);
 		ast_channel_lock(p->owner);
 		if (option_debug)
 			ast_log(LOG_DEBUG, "Detaching from %s\n", p->owner->name);
-		if (p->owner->tech_pvt)	/* i believe that's always */
-			pvt_unref(p->owner->tech_pvt);
-		p->owner->tech_pvt = NULL;
+		p->owner->tech_pvt = pvt_unref(p->owner->tech_pvt);	/* self pointer, basically */
 		ast_channel_unlock(p->owner);
 	}
 	/* Clear history */
@@ -3814,7 +3813,7 @@
 */
 static int sip_indicate(struct ast_channel *ast, int condition, const void *data, size_t datalen)
 {
-	struct sip_pvt *p = ast->tech_pvt;
+	struct sip_pvt *p = ast->tech_pvt;	/* are we sure it doesn't go away ? */
 	int res = 0;
 
 	sip_pvt_lock(p);
@@ -4248,7 +4247,7 @@
 static struct ast_frame *sip_read(struct ast_channel *ast)
 {
 	struct ast_frame *fr;
-	struct sip_pvt *p = ast->tech_pvt;
+	struct sip_pvt *p = ast->tech_pvt;	/* XXX assume it is not null ? */
 	int faxdetected = FALSE;
 
 	sip_pvt_lock(p);
@@ -11603,7 +11602,7 @@
 			number = 1;
 	}
 
-	p = chan->tech_pvt;
+	p = chan->tech_pvt;	/* chan is locked, so no need to grab a reference */
 
 	/* If there is no private structure, this channel is no longer alive */
 	if (!p) {
@@ -11769,6 +11768,7 @@
 		return -1;
 	}
 
+	/* no need to grab a reference to p because chan is locked */
 	p = chan->tech_pvt;
 
 	/* If there is no private structure, this channel is no longer alive */
@@ -11776,7 +11776,6 @@
 		ast_channel_unlock(chan);
 		return -1;
 	}
-
 	if (!strcasecmp(data, "peerip")) {
 		ast_copy_string(buf, p->sa.sin_addr.s_addr ? ast_inet_ntoa(p->sa.sin_addr) : "", len);
 	} else  if (!strcasecmp(data, "recvip")) {
@@ -13324,7 +13323,7 @@
 	sip_pvt_unlock(p);	/* Unlock SIP structure */
 
 	/* The call should be down with no ast_channel, so hang it up */
-	c->tech_pvt = NULL;
+	c->tech_pvt = NULL;	/* XXX do we need unref ? */
 	ast_hangup(c);
 	return 0;
 }