[svn-commits] mmichelson: branch mmichelson/rls-rlmi r420088 - in /team/mmichelson/rls-rlmi...

SVN commits to the Digium repositories svn-commits at lists.digium.com
Tue Aug 5 15:02:14 CDT 2014


Author: mmichelson
Date: Tue Aug  5 15:02:10 2014
New Revision: 420088

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=420088
Log:
Address the rest of the review feedback.

The biggest change here was the one getting rid of the 2048 byte stack
allocation when creating an RLMI body. Now, instead of that, I store the
XML directly in the pjsip_msg_body() and provide a callback for it to
be converted to text.

In doing so, I changed the multiple #defines of the XML prolog length
in body generators to be defined in a header instead.


Modified:
    team/mmichelson/rls-rlmi/include/asterisk/res_pjsip_presence_xml.h
    team/mmichelson/rls-rlmi/res/res_pjsip_dialog_info_body_generator.c
    team/mmichelson/rls-rlmi/res/res_pjsip_pidf_body_generator.c
    team/mmichelson/rls-rlmi/res/res_pjsip_pubsub.c
    team/mmichelson/rls-rlmi/res/res_pjsip_xpidf_body_generator.c

Modified: team/mmichelson/rls-rlmi/include/asterisk/res_pjsip_presence_xml.h
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/rls-rlmi/include/asterisk/res_pjsip_presence_xml.h?view=diff&rev=420088&r1=420087&r2=420088
==============================================================================
--- team/mmichelson/rls-rlmi/include/asterisk/res_pjsip_presence_xml.h (original)
+++ team/mmichelson/rls-rlmi/include/asterisk/res_pjsip_presence_xml.h Tue Aug  5 15:02:10 2014
@@ -15,6 +15,21 @@
  * the GNU General Public License Version 2. See the LICENSE file
  * at the top of the source tree.
  */
+
+/*!
+ * \brief The length of the XML prolog when printing
+ * presence or other XML in PJSIP.
+ *
+ * When calling any variant of pj_xml_print(), the documentation
+ * claims that it will return -1 if the provided buffer is not
+ * large enough. However, if the XML prolog is requested to be
+ * printed, then the length of the XML prolog is returned upon
+ * failure instead of -1.
+ *
+ * This constant is useful to check against when trying to determine
+ * if printing XML succeeded or failed.
+ */
+#define AST_PJSIP_XML_PROLOG_LEN 39
 
 /*!
  * PIDF state

Modified: team/mmichelson/rls-rlmi/res/res_pjsip_dialog_info_body_generator.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/rls-rlmi/res/res_pjsip_dialog_info_body_generator.c?view=diff&rev=420088&r1=420087&r2=420088
==============================================================================
--- team/mmichelson/rls-rlmi/res/res_pjsip_dialog_info_body_generator.c (original)
+++ team/mmichelson/rls-rlmi/res/res_pjsip_dialog_info_body_generator.c Tue Aug  5 15:02:10 2014
@@ -156,11 +156,6 @@
  */
 #define MAX_STRING_GROWTHS 3
 
-/* When having pj_xml_print add the XML prolog to the output body the function will return 39
- * instead of -1 if the rest of the document can not be printed into the body.
- */
-#define XML_PROLOG 39
-
 static void dialog_info_to_string(void *body, struct ast_str **str)
 {
 	pj_xml_node *dialog_info = body;
@@ -169,13 +164,13 @@
 
 	do {
 		size = pj_xml_print(dialog_info, ast_str_buffer(*str), ast_str_size(*str), PJ_TRUE);
-		if (size == XML_PROLOG) {
+		if (size == AST_PJSIP_XML_PROLOG_LEN) {
 			ast_str_make_space(str, ast_str_size(*str) * 2);
 			++growths;
 		}
-	} while (size == XML_PROLOG && growths < MAX_STRING_GROWTHS);
-
-	if (size == XML_PROLOG) {
+	} while (size == AST_PJSIP_XML_PROLOG_LEN && growths < MAX_STRING_GROWTHS);
+
+	if (size == AST_PJSIP_XML_PROLOG_LEN) {
 		ast_log(LOG_WARNING, "dialog-info+xml body text too large\n");
 		return;
 	}

Modified: team/mmichelson/rls-rlmi/res/res_pjsip_pidf_body_generator.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/rls-rlmi/res/res_pjsip_pidf_body_generator.c?view=diff&rev=420088&r1=420087&r2=420088
==============================================================================
--- team/mmichelson/rls-rlmi/res/res_pjsip_pidf_body_generator.c (original)
+++ team/mmichelson/rls-rlmi/res/res_pjsip_pidf_body_generator.c Tue Aug  5 15:02:10 2014
@@ -81,7 +81,6 @@
 }
 
 #define MAX_STRING_GROWTHS 5
-#define XML_PROLOG 39
 
 static void pidf_to_string(void *body, struct ast_str **str)
 {
@@ -91,13 +90,13 @@
 
 	do {
 		size = pjpidf_print(pres, ast_str_buffer(*str), ast_str_size(*str) - 1);
-		if (size == XML_PROLOG) {
+		if (size == AST_PJSIP_XML_PROLOG_LEN) {
 			ast_str_make_space(str, ast_str_size(*str) * 2);
 			++growths;
 		}
-	} while (size == XML_PROLOG && growths < MAX_STRING_GROWTHS);
+	} while (size == AST_PJSIP_XML_PROLOG_LEN && growths < MAX_STRING_GROWTHS);
 
-	if (size == XML_PROLOG) {
+	if (size == AST_PJSIP_XML_PROLOG_LEN) {
 		ast_log(LOG_WARNING, "PIDF body text too large\n");
 		return;
 	}

Modified: team/mmichelson/rls-rlmi/res/res_pjsip_pubsub.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/rls-rlmi/res/res_pjsip_pubsub.c?view=diff&rev=420088&r1=420087&r2=420088
==============================================================================
--- team/mmichelson/rls-rlmi/res/res_pjsip_pubsub.c (original)
+++ team/mmichelson/rls-rlmi/res/res_pjsip_pubsub.c Tue Aug  5 15:02:10 2014
@@ -1477,7 +1477,6 @@
 #endif
 	int res;
 
-	ao2_ref(sub_tree, +1);
 	res = pjsip_evsub_send_request(sub_tree->evsub, tdata) == PJ_SUCCESS ? 0 : -1;
 	subscription_persistence_update(sub_tree, NULL);
 
@@ -1486,7 +1485,6 @@
 		"Endpoint: %s\r\n",
 		pjsip_evsub_get_state_name(sub_tree->evsub),
 		ast_sorcery_object_get_id(endpoint));
-	ao2_cleanup(sub_tree);
 
 	return res;
 }
@@ -1638,6 +1636,26 @@
 	return cid;
 }
 
+static int rlmi_print_body(struct pjsip_msg_body *msg_body, char *buf, pj_size_t size)
+{
+	int num_printed;
+	pj_xml_node *rlmi = msg_body->data;
+
+	num_printed = pj_xml_print(rlmi, buf, size, PJ_TRUE);
+	if (num_printed == AST_PJSIP_XML_PROLOG_LEN) {
+		return -1;
+	}
+
+	return num_printed;
+}
+
+static void *rlmi_clone_data(pj_pool_t *pool, const void *data, unsigned len)
+{
+	const pj_xml_node *rlmi = data;
+
+	return pj_xml_clone(pool, rlmi);
+}
+
 /*!
  * \brief Create an RLMI body part for a multipart resource list body
  *
@@ -1660,11 +1678,9 @@
 	static const pj_str_t rlmi_subtype = { "rlmi+xml", 8 };
 	pj_xml_node *rlmi;
 	pj_xml_node *name;
-	pj_str_t rlmi_text;
 	pjsip_multipart_part *rlmi_part;
 	char version_str[32];
 	char uri[PJSIP_MAX_URL_SIZE];
-	char rlmi_str[2048] = { 0,};
 	pjsip_generic_string_hdr *cid;
 	int i;
 
@@ -1687,11 +1703,16 @@
 		add_rlmi_resource(pool, rlmi, part->cid, part->resource, part->uri, part->state);
 	}
 
-	pj_xml_print(rlmi, rlmi_str, sizeof(rlmi_str) - 1, PJ_TRUE);
-	pj_cstr(&rlmi_text, rlmi_str);
-
 	rlmi_part = pjsip_multipart_create_part(pool);
-	rlmi_part->body = pjsip_msg_body_create(pool, &rlmi_type, &rlmi_subtype, &rlmi_text);
+
+	rlmi_part->body = PJ_POOL_ZALLOC_T(pool, pjsip_msg_body);
+    pj_strdup(pool, &rlmi_part->body->content_type.type, &rlmi_type);
+    pj_strdup(pool, &rlmi_part->body->content_type.subtype, &rlmi_subtype);
+    pj_list_init(&rlmi_part->body->content_type.param);
+
+    rlmi_part->body->data = pj_xml_clone(pool, rlmi);
+	rlmi_part->body->clone_data = rlmi_clone_data;
+	rlmi_part->body->print_body = rlmi_print_body;
 
 	cid = generate_content_id_hdr(pool, sub);
 	pj_list_insert_before(&rlmi_part->hdr, cid);
@@ -1823,7 +1844,9 @@
 	struct body_part_list body_parts;
 	unsigned int use_full_state = force_full_state ? 1 : sub->full_state;
 
-	AST_VECTOR_INIT(&body_parts, AST_VECTOR_SIZE(&sub->children));
+	if (AST_VECTOR_INIT(&body_parts, AST_VECTOR_SIZE(&sub->children))) {
+		return NULL;
+	}
 
 	for (i = 0; i < AST_VECTOR_SIZE(&sub->children); ++i) {
 		build_body_part(pool, AST_VECTOR_GET(&sub->children, i), &body_parts, use_full_state);
@@ -1837,6 +1860,9 @@
 	multipart = create_multipart_body(pool);
 
 	rlmi_part = build_rlmi_body(pool, sub, &body_parts, use_full_state);
+	if (!rlmi_part) {
+		return NULL;
+	}
 	pjsip_multipart_add_part(pool, multipart, rlmi_part);
 
 	for (i = 0; i < AST_VECTOR_SIZE(&body_parts); ++i) {
@@ -1915,9 +1941,19 @@
 				NULL, NULL, &tdata) != PJ_SUCCESS) {
 		return -1;
 	}
+	
+	tdata->msg->body = generate_notify_body(tdata->pool, sub_tree->root, force_full_state);
+	if (!tdata->msg->body) {
+		pjsip_tx_data_dec_ref(tdata);
+		return -1;
+	}
+
+	if (sub_tree->is_list) {
+		pjsip_require_hdr *require = create_require_eventlist(tdata->pool);
+		pjsip_msg_add_hdr(tdata->msg, (pjsip_hdr *) require);
+	}
 
 	if (sip_subscription_send_request(sub_tree, tdata)) {
-		pjsip_tx_data_dec_ref(tdata);
 		return -1;
 	}
 
@@ -2893,6 +2929,14 @@
 		return;
 	}
 
+	/* If sending a NOTIFY to terminate a subscription, then pubsub_on_evsub_state()
+	 * will be called when we send the NOTIFY, and that will result in dropping the
+	 * refcount of sub_tree by one, and possibly destroying the sub_tree. We need to
+	 * hold a reference to the sub_tree until this function returns so that we don't
+	 * try to read from or write to freed memory by accident
+	 */
+	ao2_ref(sub_tree, +1);
+
 	if (pjsip_evsub_get_state(evsub) == PJSIP_EVSUB_STATE_TERMINATED) {
 		set_state_terminated(sub_tree->root);
 	}
@@ -2904,6 +2948,8 @@
 	if (sub_tree->is_list) {
 		pj_list_insert_before(res_hdr, create_require_eventlist(rdata->tp_info.pool));
 	}
+
+	ao2_ref(sub_tree, -1);
 }
 
 static void pubsub_on_rx_notify(pjsip_evsub *evsub, pjsip_rx_data *rdata, int *p_st_code,

Modified: team/mmichelson/rls-rlmi/res/res_pjsip_xpidf_body_generator.c
URL: http://svnview.digium.com/svn/asterisk/team/mmichelson/rls-rlmi/res/res_pjsip_xpidf_body_generator.c?view=diff&rev=420088&r1=420087&r2=420088
==============================================================================
--- team/mmichelson/rls-rlmi/res/res_pjsip_xpidf_body_generator.c (original)
+++ team/mmichelson/rls-rlmi/res/res_pjsip_xpidf_body_generator.c Tue Aug  5 15:02:10 2014
@@ -98,7 +98,6 @@
 }
 
 #define MAX_STRING_GROWTHS 5
-#define XML_PROLOG 39
 
 static void xpidf_to_string(void *body, struct ast_str **str)
 {
@@ -108,13 +107,13 @@
 
 	do {
 		size = pjxpidf_print(pres, ast_str_buffer(*str), ast_str_size(*str));
-		if (size == XML_PROLOG) {
+		if (size == AST_PJSIP_XML_PROLOG_LEN) {
 			ast_str_make_space(str, ast_str_size(*str) * 2);
 			++growths;
 		}
-	} while (size == XML_PROLOG && growths < MAX_STRING_GROWTHS);
+	} while (size == AST_PJSIP_XML_PROLOG_LEN && growths < MAX_STRING_GROWTHS);
 
-	if (size == XML_PROLOG) {
+	if (size == AST_PJSIP_XML_PROLOG_LEN) {
 		ast_log(LOG_WARNING, "XPIDF body text too large\n");
 		return;
 	}




More information about the svn-commits mailing list