[Asterisk-code-review] res pjsip pubsub.c: Fix body generator registration race. (asterisk[13])

[Anonymous Coward]

Anonymous Coward #1000019 has submitted this change and it was merged.

Change subject: res_pjsip_pubsub.c: Fix body generator registration race.
......................................................................


res_pjsip_pubsub.c: Fix body generator registration race.

Change-Id: Id8752073ef06472a2fd96080f4009fac42843e67
---
M res/res_pjsip_pubsub.c
1 file changed, 22 insertions(+), 14 deletions(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved
  George Joseph: Looks good to me, but someone else must approve



diff --git a/res/res_pjsip_pubsub.c b/res/res_pjsip_pubsub.c
index 6700475..e744bad 100644
--- a/res/res_pjsip_pubsub.c
+++ b/res/res_pjsip_pubsub.c
@@ -2541,20 +2541,28 @@
 	AST_RWLIST_TRAVERSE_SAFE_END;
 }
 
-static struct ast_sip_pubsub_body_generator *find_body_generator_type_subtype(const char *content_type,
-		const char *content_subtype)
+static struct ast_sip_pubsub_body_generator *find_body_generator_type_subtype_nolock(const char *type, const char *subtype)
 {
-	struct ast_sip_pubsub_body_generator *iter;
-	SCOPED_LOCK(lock, &body_generators, AST_RWLIST_RDLOCK, AST_RWLIST_UNLOCK);
+	struct ast_sip_pubsub_body_generator *gen;
 
-	AST_LIST_TRAVERSE(&body_generators, iter, list) {
-		if (!strcmp(iter->type, content_type) &&
-				!strcmp(iter->subtype, content_subtype)) {
+	AST_LIST_TRAVERSE(&body_generators, gen, list) {
+		if (!strcmp(gen->type, type)
+			&& !strcmp(gen->subtype, subtype)) {
 			break;
 		}
-	};
+	}
 
-	return iter;
+	return gen;
+}
+
+static struct ast_sip_pubsub_body_generator *find_body_generator_type_subtype(const char *type, const char *subtype)
+{
+	struct ast_sip_pubsub_body_generator *gen;
+
+	AST_RWLIST_RDLOCK(&body_generators);
+	gen = find_body_generator_type_subtype_nolock(type, subtype);
+	AST_RWLIST_UNLOCK(&body_generators);
+	return gen;
 }
 
 static struct ast_sip_pubsub_body_generator *find_body_generator_accept(const char *accept)
@@ -3092,14 +3100,14 @@
 	pj_str_t accept;
 	pj_size_t accept_len;
 
-	existing = find_body_generator_type_subtype(generator->type, generator->subtype);
+	AST_RWLIST_WRLOCK(&body_generators);
+	existing = find_body_generator_type_subtype_nolock(generator->type, generator->subtype);
 	if (existing) {
-		ast_log(LOG_WARNING, "Cannot register body generator of %s/%s."
-				"One is already registered.\n", generator->type, generator->subtype);
+		AST_RWLIST_UNLOCK(&body_generators);
+		ast_log(LOG_WARNING, "A body generator for %s/%s is already registered.\n",
+			generator->type, generator->subtype);
 		return -1;
 	}
-
-	AST_RWLIST_WRLOCK(&body_generators);
 	AST_LIST_INSERT_HEAD(&body_generators, generator, list);
 	AST_RWLIST_UNLOCK(&body_generators);
 

-- 
To view, visit https://gerrit.asterisk.org/2727
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Id8752073ef06472a2fd96080f4009fac42843e67
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>