[Asterisk-code-review] res pjsip: Update logging to show contact->uri in messages (asterisk[master])

Joshua Colp asteriskteam at digium.com
Thu Dec 3 12:39:01 CST 2015


Joshua Colp has submitted this change and it was merged.

Change subject: res_pjsip:  Update logging to show contact->uri in messages
......................................................................


res_pjsip:  Update logging to show contact->uri in messages

An earlier commit changed the id of dynamic contacts to contain
a hash instead of the uri.  This patch updates status change
logging to show the aor/uri instead of the id.  This required
adding the aor id to contact and contact_status and adding
uri to contact_status.  The aor id gets added to contact and
contact_status in their allocators and the uri gets added to
contact_status in pjsip_options when the contact_status is
created or updated.

ASTERISK-25598 #close

Reported-by: George Joseph
Tested-by: George Joseph

Change-Id: I56cbec1d2ddbe8461367dd8b6da8a6f47f6fe511
---
M include/asterisk/res_pjsip.h
M res/res_pjsip/location.c
M res/res_pjsip/pjsip_configuration.c
M res/res_pjsip/pjsip_options.c
4 files changed, 119 insertions(+), 106 deletions(-)

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



diff --git a/include/asterisk/res_pjsip.h b/include/asterisk/res_pjsip.h
index 95b8f23..d9123f9 100644
--- a/include/asterisk/res_pjsip.h
+++ b/include/asterisk/res_pjsip.h
@@ -159,6 +159,8 @@
 		AST_STRING_FIELD(path);
 		/*! Content of the User-Agent header in REGISTER request */
 		AST_STRING_FIELD(user_agent);
+		/*! The name of the aor this contact belongs to */
+		AST_STRING_FIELD(aor);
 	);
 	/*! Absolute time that this contact is no longer valid after */
 	struct timeval expiration_time;
@@ -193,6 +195,12 @@
  */
 struct ast_sip_contact_status {
 	SORCERY_OBJECT(details);
+	AST_DECLARE_STRING_FIELDS(
+		/*! The original contact's URI */
+		AST_STRING_FIELD(uri);
+		/*! The name of the aor this contact_status belongs to */
+		AST_STRING_FIELD(aor);
+	);
 	/*! Current status for a contact (default - unavailable) */
 	enum ast_sip_contact_status_type status;
 	/*! The round trip start time set before sending a qualify request */
diff --git a/res/res_pjsip/location.c b/res/res_pjsip/location.c
index 2e0d84b..4471d7c 100644
--- a/res/res_pjsip/location.c
+++ b/res/res_pjsip/location.c
@@ -99,6 +99,9 @@
 static void *contact_alloc(const char *name)
 {
 	struct ast_sip_contact *contact = ast_sorcery_generic_alloc(sizeof(*contact), contact_destroy);
+	char *id = ast_strdupa(name);
+	char *aor = id;
+	char *aor_separator = NULL;
 
 	if (!contact) {
 		return NULL;
@@ -108,6 +111,14 @@
 		ao2_cleanup(contact);
 		return NULL;
 	}
+
+	/* Dynamic contacts are delimited with ";@" and static ones with "@@" */
+	if ((aor_separator = strstr(id, ";@")) || (aor_separator = strstr(id, "@@"))) {
+		*aor_separator = '\0';
+	}
+	ast_assert(aor_separator != NULL);
+
+	ast_string_field_set(contact, aor, aor);
 
 	return contact;
 }
@@ -790,13 +801,14 @@
 	ast_assert(context->output_buffer != NULL);
 
 	indent = CLI_INDENT_TO_SPACES(context->indent_level);
-	flexwidth = CLI_LAST_TABSTOP - indent - 2;
+	flexwidth = CLI_LAST_TABSTOP - indent - 2 - strlen(contact->aor) + 1;
 
-	ast_str_append(&context->output_buffer, 0, "%*s:  %-*.*s  %-12.12s  %11.3f\n",
+	ast_str_append(&context->output_buffer, 0, "%*s:  %s/%-*.*s  %-12.12s  %11.3f\n",
 		indent,
 		"Contact",
+		contact->aor,
 		flexwidth, flexwidth,
-		wrapper->contact_id,
+		contact->uri,
 		ast_sip_get_contact_short_status_label(status ? status->status : UNKNOWN),
 		(status && (status->status != UNKNOWN) ? ((long long) status->rtt) / 1000.0 : NAN));
 
diff --git a/res/res_pjsip/pjsip_configuration.c b/res/res_pjsip/pjsip_configuration.c
index 3a6f9d7..30e5674 100644
--- a/res/res_pjsip/pjsip_configuration.c
+++ b/res/res_pjsip/pjsip_configuration.c
@@ -56,47 +56,37 @@
 	return !strcmp(ast_endpoint_get_resource(persistent1->endpoint), id) ? CMP_MATCH | CMP_STOP : 0;
 }
 
-/*! \brief Structure for communicating contact status to
- * persistent_endpoint_update_state from the contact/contact_status
- * observers.
- */
-struct sip_contact_status {
-	char *uri;
-	enum ast_sip_contact_status_type status;
-	int64_t rtt;
-};
-
 /*! \brief Callback function for changing the state of an endpoint */
-static int persistent_endpoint_update_state(void *obj, void *arg, void *data, int flags)
+static int persistent_endpoint_update_state(void *obj, void *arg, int flags)
 {
 	struct sip_persistent_endpoint *persistent = obj;
 	struct ast_endpoint *endpoint = persistent->endpoint;
-	char *aor = arg;
-	struct sip_contact_status *status = data;
+	struct ast_sip_contact_status *status = arg;
 	struct ao2_container *contacts;
 	struct ast_json *blob;
 	struct ao2_iterator i;
 	struct ast_sip_contact *contact;
 	enum ast_endpoint_state state = AST_ENDPOINT_OFFLINE;
 
-	if (!ast_strlen_zero(aor)) {
-		if (!strstr(persistent->aors, aor)) {
+	if (status) {
+		char rtt[32];
+
+		/* If the status' aor isn't one of the endpoint's, we skip */
+		if (!strstr(persistent->aors, status->aor)) {
 			return 0;
 		}
 
-		if (status) {
-			char rtt[32];
-			snprintf(rtt, 31, "%" PRId64, status->rtt);
-			blob = ast_json_pack("{s: s, s: s, s: s, s: s, s: s}",
-				"contact_status", ast_sip_get_contact_status_label(status->status),
-				"aor", aor,
-				"uri", status->uri,
-				"roundtrip_usec", rtt,
-				"endpoint_name", ast_endpoint_get_resource(endpoint));
-			ast_endpoint_blob_publish(endpoint, ast_endpoint_contact_state_type(), blob);
-			ast_json_unref(blob);
-		}
+		snprintf(rtt, sizeof(rtt), "%" PRId64, status->rtt);
+		blob = ast_json_pack("{s: s, s: s, s: s, s: s, s: s}",
+			"contact_status", ast_sip_get_contact_status_label(status->status),
+			"aor", status->aor,
+			"uri", status->uri,
+			"roundtrip_usec", rtt,
+			"endpoint_name", ast_endpoint_get_resource(endpoint));
+		ast_endpoint_blob_publish(endpoint, ast_endpoint_contact_state_type(), blob);
+		ast_json_unref(blob);
 	}
+
 	/* Find all the contacts for this endpoint.  If ANY are available,
 	 * mark the endpoint as ONLINE.
 	 */
@@ -142,57 +132,50 @@
 	return 0;
 }
 
-/*! \brief Function called when stuff relating to a contact happens (created/deleted) */
+/*! \brief Function called when a contact is created */
 static void persistent_endpoint_contact_created_observer(const void *object)
 {
 	const struct ast_sip_contact *contact = object;
-	char *id = ast_strdupa(ast_sorcery_object_get_id(contact));
-	char *aor = NULL;
-	char *contact_uri = NULL;
-	struct sip_contact_status status;
+	struct ast_sip_contact_status *contact_status;
 
-	aor = id;
-	/* Dynamic contacts are delimited with ";@" and static ones with "@@" */
-	if ((contact_uri = strstr(id, ";@")) || (contact_uri = strstr(id, "@@"))) {
-		*contact_uri = '\0';
-		contact_uri += 2;
-	} else {
-		contact_uri = id;
+	contact_status = ast_sorcery_alloc(ast_sip_get_sorcery(), CONTACT_STATUS,
+		ast_sorcery_object_get_id(contact));
+	if (!contact_status) {
+		ast_log(LOG_ERROR, "Unable to create ast_sip_contact_status for contact %s/%s\n",
+			contact->aor, contact->uri);
+		return;
 	}
+	ast_string_field_set(contact_status, uri, contact->uri);
 
-	status.uri = contact_uri;
-	status.status = CREATED;
-	status.rtt = 0;
+	contact_status->status = CREATED;
 
-	ast_verb(1, "Contact %s/%s has been created\n", aor, contact_uri);
+	ast_verb(1, "Contact %s/%s has been created\n",contact->aor, contact->uri);
 
-	ao2_callback_data(persistent_endpoints, OBJ_NODATA, persistent_endpoint_update_state, aor, &status);
+	ao2_callback(persistent_endpoints, OBJ_NODATA, persistent_endpoint_update_state, contact_status);
+	ao2_cleanup(contact_status);
 }
 
-/*! \brief Function called when stuff relating to a contact happens (created/deleted) */
+/*! \brief Function called when a contact is deleted */
 static void persistent_endpoint_contact_deleted_observer(const void *object)
 {
-	char *id = ast_strdupa(ast_sorcery_object_get_id(object));
-	char *aor = NULL;
-	char *contact_uri = NULL;
-	struct sip_contact_status status;
+	const struct ast_sip_contact *contact = object;
+	struct ast_sip_contact_status *contact_status;
 
-	aor = id;
-	/* Dynamic contacts are delimited with ";@" and static ones with "@@" */
-	if ((contact_uri = strstr(id, ";@")) || (contact_uri = strstr(id, "@@"))) {
-		*contact_uri = '\0';
-		contact_uri += 2;
-	} else {
-		contact_uri = id;
+	contact_status = ast_sorcery_alloc(ast_sip_get_sorcery(), CONTACT_STATUS,
+		ast_sorcery_object_get_id(contact));
+	if (!contact_status) {
+		ast_log(LOG_ERROR, "Unable to create ast_sip_contact_status for contact %s/%s\n",
+			contact->aor, contact->uri);
+		return;
 	}
+	ast_string_field_set(contact_status, uri, contact->uri);
 
-	ast_verb(1, "Contact %s/%s has been deleted\n", aor, contact_uri);
+	contact_status->status = REMOVED;
 
-	status.uri = contact_uri;
-	status.status = REMOVED;
-	status.rtt = 0;
+	ast_verb(1, "Contact %s/%s has been deleted\n", contact->aor, contact->uri);
 
-	ao2_callback_data(persistent_endpoints, OBJ_NODATA, persistent_endpoint_update_state, aor, &status);
+	ao2_callback(persistent_endpoints, OBJ_NODATA, persistent_endpoint_update_state, contact_status);
+	ao2_cleanup(contact_status);
 }
 
 /*! \brief Observer for contacts so state can be updated on respective endpoints */
@@ -201,36 +184,23 @@
 	.deleted = persistent_endpoint_contact_deleted_observer,
 };
 
-/*! \brief Function called when stuff relating to a contact status happens (updated) */
+/*! \brief Function called when a contact_status is updated */
 static void persistent_endpoint_contact_status_observer(const void *object)
 {
-	const struct ast_sip_contact_status *contact_status = object;
-	char *id = ast_strdupa(ast_sorcery_object_get_id(object));
-	char *aor = NULL;
-	char *contact_uri = NULL;
-	struct sip_contact_status status;
+	struct ast_sip_contact_status *contact_status = (struct ast_sip_contact_status *)object;
 
 	/* If rtt_start is set (this is the outgoing OPTIONS), ignore. */
 	if (contact_status->rtt_start.tv_sec > 0) {
 		return;
 	}
 
-	aor = id;
-	/* Dynamic contacts are delimited with ";@" and static ones with "@@" */
-	if ((contact_uri = strstr(id, ";@")) || (contact_uri = strstr(id, "@@"))) {
-		*contact_uri = '\0';
-		contact_uri += 2;
-	} else {
-		contact_uri = id;
-	}
-
 	if (contact_status->status == contact_status->last_status) {
-		ast_debug(3, "Contact %s status didn't change: %s, RTT: %.3f msec\n",
-			contact_uri, ast_sip_get_contact_status_label(contact_status->status),
+		ast_debug(3, "Contact %s/%s status didn't change: %s, RTT: %.3f msec\n",
+			contact_status->aor, contact_status->uri, ast_sip_get_contact_status_label(contact_status->status),
 			contact_status->rtt / 1000.0);
 		return;
 	} else {
-		ast_verb(1, "Contact %s/%s is now %s.  RTT: %.3f msec\n", aor, contact_uri,
+		ast_verb(1, "Contact %s/%s is now %s.  RTT: %.3f msec\n", contact_status->aor, contact_status->uri,
 			ast_sip_get_contact_status_label(contact_status->status),
 			contact_status->rtt / 1000.0);
 	}
@@ -241,11 +211,7 @@
 		ast_sorcery_object_get_id(contact_status),
 		ast_sip_get_contact_status_label(contact_status->status));
 
-	status.uri = contact_uri;
-	status.status = contact_status->status;
-	status.rtt = contact_status->rtt;
-
-	ao2_callback_data(persistent_endpoints, OBJ_NODATA, persistent_endpoint_update_state, aor, &status);
+	ao2_callback(persistent_endpoints, OBJ_NODATA, persistent_endpoint_update_state, contact_status);
 }
 
 /*! \brief Observer for contacts so state can be updated on respective endpoints */
@@ -1085,7 +1051,7 @@
 		if (ast_strlen_zero(persistent->aors)) {
 			ast_endpoint_set_state(persistent->endpoint, AST_ENDPOINT_UNKNOWN);
 		} else {
-			persistent_endpoint_update_state(persistent, NULL, NULL, 0);
+			persistent_endpoint_update_state(persistent, NULL, 0);
 		}
 
 		ao2_link_flags(persistent_endpoints, persistent, OBJ_NOLOCK);
diff --git a/res/res_pjsip/pjsip_options.c b/res/res_pjsip/pjsip_options.c
index d55a995..6762b75 100644
--- a/res/res_pjsip/pjsip_options.c
+++ b/res/res_pjsip/pjsip_options.c
@@ -42,7 +42,6 @@
 	[UNKNOWN] = "Unknown",
 	[CREATED] = "Created",
 	[REMOVED] = "Removed",
-
 };
 
 static const char *short_status_map [] = {
@@ -65,18 +64,45 @@
 
 /*!
  * \internal
+ * \brief Destroy a ast_sip_contact_status object.
+ */
+static void contact_status_destroy(void * obj)
+{
+	struct ast_sip_contact_status *status = obj;
+
+	ast_string_field_free_memory(status);
+}
+
+/*!
+ * \internal
  * \brief Create a ast_sip_contact_status object.
  */
 static void *contact_status_alloc(const char *name)
 {
-	struct ast_sip_contact_status *status = ast_sorcery_generic_alloc(sizeof(*status), NULL);
+	struct ast_sip_contact_status *status = ast_sorcery_generic_alloc(sizeof(*status), contact_status_destroy);
+	char *id = ast_strdupa(name);
+	char *aor = id;
+	char *aor_separator = NULL;
 
 	if (!status) {
 		ast_log(LOG_ERROR, "Unable to allocate ast_sip_contact_status\n");
 		return NULL;
 	}
 
-	status->status = UNKNOWN;
+	if (ast_string_field_init(status, 256)) {
+		ast_log(LOG_ERROR, "Unable to allocate ast_sip_contact_status stringfields\n");
+		ao2_cleanup(status);
+		return NULL;
+	}
+
+	/* Dynamic contacts are delimited with ";@" and static ones with "@@" */
+	if ((aor_separator = strstr(id, ";@")) || (aor_separator = strstr(id, "@@"))) {
+		*aor_separator = '\0';
+	}
+	ast_assert(aor_separator != NULL);
+
+	ast_string_field_set(status, aor, aor);
+	status->status = CREATED;
 
 	return status;
 }
@@ -98,12 +124,12 @@
 	status = ast_sorcery_alloc(ast_sip_get_sorcery(), CONTACT_STATUS,
 		ast_sorcery_object_get_id(contact));
 	if (!status) {
-		ast_log(LOG_ERROR, "Unable to create ast_sip_contact_status for contact %s\n",
-			contact->uri);
+		ast_log(LOG_ERROR, "Unable to create ast_sip_contact_status for contact %s/%s\n",
+			contact->aor, contact->uri);
 		return NULL;
 	}
 
-	status->status = UNKNOWN;
+	ast_string_field_set(status, uri, contact->uri);
 	status->rtt_start = ast_tv(0, 0);
 	status->rtt = 0;
 
@@ -127,8 +153,8 @@
 static void update_contact_status(const struct ast_sip_contact *contact,
 	enum ast_sip_contact_status_type value)
 {
-	struct ast_sip_contact_status *status;
-	struct ast_sip_contact_status *update;
+	RAII_VAR(struct ast_sip_contact_status *, status, NULL, ao2_cleanup);
+	RAII_VAR(struct ast_sip_contact_status *, update, NULL, ao2_cleanup);
 
 	status = ast_res_pjsip_find_or_create_contact_status(contact);
 	if (!status) {
@@ -145,6 +171,7 @@
 		return;
 	}
 
+	ast_string_field_set(update, uri, contact->uri);
 	update->last_status = status->status;
 	update->status = value;
 	if (update->last_status != update->status) {
@@ -175,9 +202,6 @@
 		ast_log(LOG_ERROR, "Unable to update ast_sip_contact_status for contact %s\n",
 			contact->uri);
 	}
-
-	ao2_ref(status, -1);
-	ao2_ref(update, -1);
 }
 
 /*!
@@ -187,8 +211,8 @@
  */
 static void init_start_time(const struct ast_sip_contact *contact)
 {
-	struct ast_sip_contact_status *status;
-	struct ast_sip_contact_status *update;
+	RAII_VAR(struct ast_sip_contact_status *, status, NULL, ao2_cleanup);
+	RAII_VAR(struct ast_sip_contact_status *, update, NULL, ao2_cleanup);
 
 	status = ast_res_pjsip_find_or_create_contact_status(contact);
 	if (!status) {
@@ -205,6 +229,7 @@
 		return;
 	}
 
+	ast_string_field_set(status, uri, contact->uri);
 	update->status = status->status;
 	update->last_status = status->last_status;
 	update->rtt = status->rtt;
@@ -214,9 +239,6 @@
 		ast_log(LOG_ERROR, "Unable to update ast_sip_contact_status for contact %s\n",
 			contact->uri);
 	}
-
-	ao2_ref(status, -1);
-	ao2_ref(update, -1);
 }
 
 /*!
@@ -993,6 +1015,9 @@
 	return 0;
 }
 
+static char status_value_unknown[2];
+static char status_value_created[2];
+
 int ast_sip_initialize_sorcery_qualify(void)
 {
 	struct ast_sorcery *sorcery = ast_sip_get_sorcery();
@@ -1006,10 +1031,12 @@
 		return -1;
 	}
 
+	snprintf(status_value_unknown, sizeof(status_value_unknown), "%u", UNKNOWN);
 	ast_sorcery_object_field_register_nodoc(sorcery, CONTACT_STATUS, "last_status",
-		"0", OPT_UINT_T, 1, FLDSET(struct ast_sip_contact_status, last_status));
+		status_value_unknown, OPT_UINT_T, 1, FLDSET(struct ast_sip_contact_status, last_status));
+	snprintf(status_value_created, sizeof(status_value_created), "%u", CREATED);
 	ast_sorcery_object_field_register_nodoc(sorcery, CONTACT_STATUS, "status",
-		"0", OPT_UINT_T, 1, FLDSET(struct ast_sip_contact_status, status));
+		status_value_created, OPT_UINT_T, 1, FLDSET(struct ast_sip_contact_status, status));
 	ast_sorcery_object_field_register_custom_nodoc(sorcery, CONTACT_STATUS, "rtt_start",
 		"0.0", rtt_start_handler, rtt_start_to_str, NULL, 0, 0);
 	ast_sorcery_object_field_register_nodoc(sorcery, CONTACT_STATUS, "rtt",

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I56cbec1d2ddbe8461367dd8b6da8a6f47f6fe511
Gerrit-PatchSet: 7
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>



More information about the asterisk-code-review mailing list