[svn-commits] rizzo: branch rizzo/astobj2 r47356 - /team/rizzo/astobj2/channels/chan_sip.c

[svn-commits at lists.digium.com]

Author: rizzo
Date: Thu Nov  9 04:40:57 2006
New Revision: 47356

URL: http://svn.digium.com/view/asterisk?view=rev&rev=47356
Log:
make sure that all fields of a sip_pvt are properly
freed if an error occurs during sip_alloc().
The current code leaks the string field and maybe something else.

trunk and 1.4 candidate (with proper modifications), if approved.


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=47356&r1=47355&r2=47356
==============================================================================
--- team/rizzo/astobj2/channels/chan_sip.c (original)
+++ team/rizzo/astobj2/channels/chan_sip.c Thu Nov  9 04:40:57 2006
@@ -1011,6 +1011,8 @@
 #define	CMP_MATCH	1
 #define	CMP_STOP	2
 
+#define ao2_ref(x, n)	/* nothing here */
+
 static struct sip_pvt *dialoglist = NULL;
 
 /*! \brief Protect the SIP dialog list (of sip_pvt's) */
@@ -1027,6 +1029,16 @@
 	ast_mutex_unlock(&dialoglock);
 }
 #endif
+
+static void pvt_ref(struct sip_pvt *p)
+{
+	ao2_ref(p, 1);
+}
+
+static void pvt_unref(struct sip_pvt *p)
+{
+	ao2_ref(p, -1);
+}
 
 #define FLAG_RESPONSE (1 << 0)
 #define FLAG_FATAL (1 << 1)
@@ -1299,7 +1311,7 @@
 static void sip_scheddestroy(struct sip_pvt *p, int ms);
 static void sip_cancel_destroy(struct sip_pvt *p);
 static void sip_destroy(struct sip_pvt *p);
-static void __sip_destroy(struct sip_pvt *p);
+static struct sip_pvt *__sip_destroy(struct sip_pvt *p);
 static void __sip_ack(struct sip_pvt *p, int seqno, int resp, int sipmethod, int reset);
 static void __sip_pretend_ack(struct sip_pvt *p);
 static int __sip_semi_ack(struct sip_pvt *p, int seqno, int resp, int sipmethod);
@@ -3037,8 +3049,9 @@
 /*! \brief Execute destruction of SIP dialog structure, release memory.
  * Assume the pvt has been already extracted from the list.
  */
-static void __sip_destroy(struct sip_pvt *p)
-{
+static void pvt_destructor(void *__p)
+{
+	struct sip_pvt *p = __p;
 	struct sip_pkt *cp;
 
 	if (sip_debug_test_pvt(p) || option_debug > 2)
@@ -3109,11 +3122,17 @@
 	}
 
 	ast_string_field_free_pools(p);
-
+}
+
+static struct sip_pvt *__sip_destroy(struct sip_pvt *p)
+{
+	ast_mutex_destroy(&p->pvt_lock);	/* XXX not used in ao2 */
+	pvt_unref(p);	/* automatically calls pvt_destructor on ao2*/
 #ifndef USE_AO2
-	ast_mutex_destroy(&p->pvt_lock);
+	pvt_destructor(p);
 	free(p);
 #endif
+	return NULL;
 }
 
 /*! \brief  update_call_counter: Handle call_limit for SIP users 
@@ -4288,14 +4307,12 @@
 	snprintf(tagbuf, len, "as%08lx", ast_random());
 }
 
-#ifdef USE_AO2
-static void pvt_destructor(void *p)
-{
-	__sip_destroy(p);
-}
-#endif
-
-/*! \brief Allocate SIP_PVT structure and set defaults */
+
+/*!
+ * Allocate SIP_PVT structure, set defaults and store in the container.
+ * Returns a reference to the object so whoever uses it later must
+ * remember to release the reference.
+ */
 static struct sip_pvt *sip_alloc(ast_string_field callid, struct sockaddr_in *sin,
 				 int useglobal_nat, const int intended_method)
 {
@@ -4303,30 +4320,26 @@
 
 #ifdef USE_AO2
 	p = ao2_alloc(sizeof(*p), pvt_destructor);
+#else
+	p = ast_calloc(1, sizeof(*p));
+#endif
 	if (!p)
 		return NULL;
-	if (ast_string_field_init(p, 512)) {
-		ao2_ref(p, -1);
-		return NULL;
-	}
-#else
-	if (!(p = ast_calloc(1, sizeof(*p))))
-		return NULL;
-
-	if (ast_string_field_init(p, 512)) {
-		free(p);
-		return NULL;
-	}
-
-	ast_mutex_init(&p->pvt_lock);
-#endif
-
+
+	ast_mutex_init(&p->pvt_lock);	/* XXX not necessary in ao2 */
+
+	/* initialize fields so that __sip_destroy() can handle them */
 	p->method = intended_method;
 	p->initid = -1;
 	p->autokillid = -1;
 	p->subscribed = NONE;
 	p->stateid = -1;
 	p->prefs = default_prefs;		/* Set default codecs for this call */
+
+	/* from now on we can safely call __sip_destroy() to free resources in case of errors */
+
+	if (ast_string_field_init(p, 512))
+		return __sip_destroy(p);
 
 	if (intended_method != SIP_OPTIONS)	/* Peerpoke has it's own system */
 		p->timer_t1 = SIP_TIMER_T1;	/* Default SIP retransmission timer T1 (RFC 3261) */
@@ -4358,13 +4371,7 @@
 		if (!p->rtp || (ast_test_flag(&p->flags[1], SIP_PAGE2_VIDEOSUPPORT) && !p->vrtp)) {
 			ast_log(LOG_WARNING, "Unable to create RTP audio %s session: %s\n",
 				ast_test_flag(&p->flags[1], SIP_PAGE2_VIDEOSUPPORT) ? "and video" : "", strerror(errno));
-			ast_mutex_destroy(&p->pvt_lock);
-			if (p->chanvars) {
-				ast_variables_destroy(p->chanvars);
-				p->chanvars = NULL;
-			}
-			free(p);
-			return NULL;
+			return __sip_destroy(p);
 		}
 		ast_rtp_setdtmf(p->rtp, ast_test_flag(&p->flags[0], SIP_DTMF) != SIP_DTMF_INFO);
 		ast_rtp_setdtmfcompensate(p->rtp, ast_test_flag(&p->flags[1], SIP_PAGE2_RFC2833_COMPENSATE));
@@ -4417,6 +4424,7 @@
 	}
 	ast_string_field_set(p, context, default_context);
 
+	pvt_ref(p);	/* prepare to return a reference */
 	/* Add to active dialog list */
 #ifdef USE_AO2
 	ao2_link(dialogs, p);