[Asterisk-code-review] res calendar: On reload, update all configuration (asterisk[14])

[Sean Bright]

Sean Bright has uploaded this change for review. ( https://gerrit.asterisk.org/6508


Change subject: res_calendar: On reload, update all configuration
......................................................................

res_calendar: On reload, update all configuration

This changes the behavior of res_calendar to drop all existing calendars
and re-create them whenever a reload is done. The Calendar API provides
no way for configuration information to be pushed down to calendar
'techs' so updated settings would not take affect until a module
unload/load was done or Asterisk was restarted.

Also fix a tiny memory leak in res_calendar_caldav while we're at it.

ASTERISK-25524 #close
Reported by: Jesper

Change-Id: Ib0f8057642e9d471960f1a79fd42e5a3ce587d3b
---
M include/asterisk/calendar.h
M res/res_calendar.c
M res/res_calendar_caldav.c
3 files changed, 29 insertions(+), 56 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/08/6508/1

diff --git a/include/asterisk/calendar.h b/include/asterisk/calendar.h
index da4af01..57140ba 100644
--- a/include/asterisk/calendar.h
+++ b/include/asterisk/calendar.h
@@ -133,7 +133,7 @@
 	pthread_t thread;    /*!< The thread that the calendar is loaded/updated in */
 	ast_cond_t unload;
 	int unloading:1;
-	int pending_deletion:1;
+	int pending_deletion:1; /*!< No longer used */
 	struct ao2_container *events;  /*!< The events that are known at this time */
 };
 
diff --git a/res/res_calendar.c b/res/res_calendar.c
index 5213d05..089fb98 100644
--- a/res/res_calendar.c
+++ b/res/res_calendar.c
@@ -341,10 +341,7 @@
 	}
 	ast_calendar_clear_events(cal);
 	ast_string_field_free_memory(cal);
-	if (cal->vars) {
-		ast_variables_destroy(cal->vars);
-		cal->vars = NULL;
-	}
+	ast_variables_destroy(cal->vars);
 	ao2_ref(cal->events, -1);
 	ao2_unlock(cal);
 }
@@ -406,28 +403,22 @@
 {
 	struct ast_calendar *cal;
 	struct ast_variable *v, *last = NULL;
-	int new_calendar = 0;
 
-	if (!(cal = find_calendar(cat))) {
-		new_calendar = 1;
-		if (!(cal = ao2_alloc(sizeof(*cal), calendar_destructor))) {
-			ast_log(LOG_ERROR, "Could not allocate calendar structure. Stopping.\n");
-			return NULL;
-		}
+	if (!(cal = ao2_alloc(sizeof(*cal), calendar_destructor))) {
+		ast_log(LOG_ERROR, "Could not allocate calendar structure. Stopping.\n");
+		return NULL;
+	}
 
-		if (!(cal->events = ao2_container_alloc(CALENDAR_BUCKETS, event_hash_fn, event_cmp_fn))) {
-			ast_log(LOG_ERROR, "Could not allocate events container for %s\n", cat);
-			cal = unref_calendar(cal);
-			return NULL;
-		}
+	if (!(cal->events = ao2_container_alloc(CALENDAR_BUCKETS, event_hash_fn, event_cmp_fn))) {
+		ast_log(LOG_ERROR, "Could not allocate events container for %s\n", cat);
+		cal = unref_calendar(cal);
+		return NULL;
+	}
 
-		if (ast_string_field_init(cal, 32)) {
-			ast_log(LOG_ERROR, "Couldn't create string fields for %s\n", cat);
-			cal = unref_calendar(cal);
-			return NULL;
-		}
-	} else {
-		cal->pending_deletion = 0;
+	if (ast_string_field_init(cal, 32)) {
+		ast_log(LOG_ERROR, "Couldn't create string fields for %s\n", cat);
+		cal = unref_calendar(cal);
+		return NULL;
 	}
 
 	ast_string_field_set(cal, name, cat);
@@ -489,17 +480,15 @@
 				cal->name);
 	}
 
-	if (new_calendar) {
-		cal->thread = AST_PTHREADT_NULL;
-		ast_cond_init(&cal->unload, NULL);
-		ao2_link(calendars, cal);
-		if (ast_pthread_create(&cal->thread, NULL, cal->tech->load_calendar, cal)) {
-			/* If we start failing to create threads, go ahead and return NULL
-			 * and the tech module will be unregistered
-			 */
-			ao2_unlink(calendars, cal);
-			cal = unref_calendar(cal);
-		}
+	cal->thread = AST_PTHREADT_NULL;
+	ast_cond_init(&cal->unload, NULL);
+	ao2_link(calendars, cal);
+	if (ast_pthread_create(&cal->thread, NULL, cal->tech->load_calendar, cal)) {
+		/* If we start failing to create threads, go ahead and return NULL
+		 * and the tech module will be unregistered
+		 */
+		ao2_unlink(calendars, cal);
+		cal = unref_calendar(cal);
 	}
 
 	return cal;
@@ -1770,30 +1759,16 @@
 	.read = calendar_event_read,
 };
 
-static int cb_pending_deletion(void *user_data, void *arg, int flags)
-{
-	struct ast_calendar *cal = user_data;
-
-	cal->pending_deletion = 1;
-
-	return CMP_MATCH;
-}
-
-static int cb_rm_pending_deletion(void *user_data, void *arg, int flags)
-{
-	struct ast_calendar *cal = user_data;
-
-	return cal->pending_deletion ? CMP_MATCH : 0;
-}
-
 static int reload(void)
 {
 	struct ast_calendar_tech *iter;
 
 	ast_mutex_lock(&reloadlock);
 
-	/* Mark existing calendars for deletion */
-	ao2_callback(calendars, OBJ_NODATA | OBJ_MULTIPLE, cb_pending_deletion, NULL);
+	/* Delete all of the calendars */
+	ao2_callback(calendars, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, NULL, NULL);
+
+	/* Load configuration */
 	load_config(1);
 
 	AST_LIST_LOCK(&techs);
@@ -1803,9 +1778,6 @@
 		}
 	}
 	AST_LIST_UNLOCK(&techs);
-
-	/* Delete calendars that no longer show up in the config */
-	ao2_callback(calendars, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, cb_rm_pending_deletion, NULL);
 
 	ast_mutex_unlock(&reloadlock);
 
diff --git a/res/res_calendar_caldav.c b/res/res_calendar_caldav.c
index 1a4b52b..9b76794 100644
--- a/res/res_calendar_caldav.c
+++ b/res/res_calendar_caldav.c
@@ -80,6 +80,7 @@
 	if (pvt->session) {
 		ne_session_destroy(pvt->session);
 	}
+	ne_uri_free(&pvt->uri);
 	ast_string_field_free_memory(pvt);
 
 	ao2_callback(pvt->events, OBJ_UNLINK | OBJ_NODATA | OBJ_MULTIPLE, NULL, NULL);

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

Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib0f8057642e9d471960f1a79fd42e5a3ce587d3b
Gerrit-Change-Number: 6508
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170915/b9597ef9/attachment-0001.html>