[asterisk-commits] twilson: branch twilson/config_work r366910 - in /team/twilson/config_work: a...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri May 18 09:37:39 CDT 2012


Author: twilson
Date: Fri May 18 09:37:35 2012
New Revision: 366910

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=366910
Log:
OMGOMGOMG this is soo much better

No more shall there be separate private and private config containers.

Just because you can't store the config data on the "private" object
(because a failure in parsing the config would mean that you've updated
some of the privates, but not others after a reload), it doesn't mean
that you can't store a pointer to the private on the config data (which
is recreated from scratch after every reload, and therefor not a problem).

Basically, instead of separating out the config from the private state,
I now realize that the "private" (which I'm totally renaming "item" later
because private is a horrible name that confuses people because it means
something else in Asterisk) *is* the config data, and that any state
that must exist after reload is the object that must be "separated".

Since the state object is an refcounted object, when allocing a private
config object (item!), if there is an existing object, we can just keep
the old state around, reffing it. The code ends up being much simpler.

</ramble>

Modified:
    team/twilson/config_work/apps/app_skel.c
    team/twilson/config_work/include/asterisk/config_options.h
    team/twilson/config_work/main/config_options.c

Modified: team/twilson/config_work/apps/app_skel.c
URL: http://svnview.digium.com/svn/asterisk/team/twilson/config_work/apps/app_skel.c?view=diff&rev=366910&r1=366909&r2=366910
==============================================================================
--- team/twilson/config_work/apps/app_skel.c (original)
+++ team/twilson/config_work/apps/app_skel.c Fri May 18 09:37:35 2012
@@ -111,6 +111,10 @@
 	struct ast_sockaddr bindaddr;
 };
 
+struct skel_pvt {
+	size_t non_config_state;
+};
+
 struct skel_pvt_config {
 	AST_DECLARE_STRING_FIELDS(
 		AST_STRING_FIELD(name);
@@ -121,27 +125,19 @@
 	struct ast_ha *permit;
 	unsigned int bit1:1;
 	unsigned int bit2:1;
-};
-
-struct skel_pvt {
-	AST_DECLARE_STRING_FIELDS(
-		AST_STRING_FIELD(name);
-	);
-	size_t non_config_state;
+	struct skel_pvt *pvt;
 };
 
 #define PVT_BUCKETS 17
 
 struct skel_config {
 	struct skel_global_config *general;
-	struct ao2_container *pvts;
 	struct ao2_container *cfgs;
 };
 
 static void *skel_config_alloc(void);
 static void *skel_pvt_cfg_alloc(const char *cat);
-static void *skel_find_or_create_pvt(const char *category);
-static void *skel_find_pvt(struct ao2_container *tmp_container, const char *category);
+static int skel_cfg_exists(struct ao2_container *tmp_container, const char *category);
 
 static struct aco_type general_options = {
 	.type = ACO_GLOBAL_OBJ,
@@ -155,9 +151,7 @@
 	.category_allow = ACO_BLACKLIST,
 	.category = "general",
 	.cfg_alloc = skel_pvt_cfg_alloc,
-	.find_or_create_pvt = skel_find_or_create_pvt,
-	.find_pvt = skel_find_pvt,
-	.pvt_offset = offsetof(struct skel_config, pvts),
+	.cfg_exists = skel_cfg_exists,
 	.cfg_offset = offsetof(struct skel_config, cfgs),
 };
 
@@ -173,52 +167,16 @@
 	ast_string_field_free_memory(cfg);
 }
 
-static void skel_pvt_cfg_unlink(struct skel_pvt_config *pcfg)
-{
-	RAII_VAR(struct skel_config *, cfg, ao2_global_obj_ref(globals), ao2_cleanup);
-	if (cfg && cfg->cfgs) {
-		ao2_unlink(cfg->cfgs, pcfg);
-	}
-}
-
-static struct skel_pvt_config *skel_pvt_find_cfg(struct skel_pvt *pvt)
-{
-	RAII_VAR(struct skel_config *, cfg, ao2_global_obj_ref(globals), ao2_cleanup);
-	if (cfg && cfg->cfgs) {
-		return ao2_find(cfg->cfgs, pvt->name, OBJ_KEY);
-	}
-
-	return NULL;
-}
-
 static void skel_pvt_destructor(void *obj)
 {
-	struct skel_pvt *pvt = obj;
-	RAII_VAR(struct skel_pvt_config *, cfg, skel_pvt_find_cfg(pvt), ao2_cleanup);
-	if (cfg) {
-		skel_pvt_cfg_unlink(cfg);
-	}
-	ast_string_field_free_memory(pvt);
-}
-
-static int skel_pvt_hash(const void *obj, const int flags)
-{
-	const struct skel_pvt *pvt = obj;
-	const char *name = (flags & OBJ_KEY) ? obj : pvt->name;
-	return ast_str_case_hash(name);
-}
-
-static int skel_pvt_cmp(void *obj, void *arg, int flags)
-{
-	struct skel_pvt *one = obj, *two = arg;
-	const char *match = (flags & OBJ_KEY) ? arg : two->name;
-	return strcasecmp(one->name, match) ? 0 : (CMP_MATCH | CMP_STOP);
+	return;
 }
 
 static void skel_pvt_config_destructor(void *obj)
 {
 	struct skel_pvt_config *cfg = obj;
 	ast_string_field_free_memory(cfg);
+	ao2_cleanup(cfg->pvt);
 }
 
 static int skel_pvt_config_hash(const void *obj, const int flags)
@@ -307,27 +265,24 @@
 		return NULL;
 	}
 
-	if (ast_string_field_init(pvt, 128)) {
-		ao2_ref(pvt, -1);
-		return NULL;
-	}
-	ast_string_field_set(pvt, name, name);
 	return pvt;
 }
 
-static void *skel_find_pvt(struct ao2_container *tmp_container, const char *category)
-{
-	return ao2_find(tmp_container, category, OBJ_KEY);
+static int skel_cfg_exists(struct ao2_container *tmp_container, const char *category)
+{
+	RAII_VAR(struct skel_pvt_config *, cfg, ao2_find(tmp_container, category, OBJ_KEY), ao2_cleanup);
+	return cfg ? 1 : 0;
 }
 
 static void *skel_find_or_create_pvt(const char *category)
 {
 	RAII_VAR(struct skel_config *, cfg, ao2_global_obj_ref(globals), ao2_cleanup);
-	void *obj;
-	if (!cfg || !cfg->pvts || !(obj = ao2_find(cfg->pvts, category, OBJ_KEY))) {
+	RAII_VAR(struct skel_pvt_config *, obj, NULL, ao2_cleanup);
+	if (!cfg || !cfg->cfgs || !(obj = ao2_find(cfg->cfgs, category, OBJ_KEY))) {
 		return skel_pvt_alloc(category);
 	}
-	return obj;
+	ao2_ref(obj->pvt, +1);
+	return obj->pvt;
 }
 
 static void *skel_pvt_cfg_alloc(const char *cat)
@@ -348,6 +303,11 @@
 		return NULL;
 	}
 
+	if (!(pvtcfg->pvt = skel_find_or_create_pvt(cat))) {
+		ao2_ref(pvtcfg, -1);
+		return NULL;
+	}
+
 	ast_string_field_set(pvtcfg, name, cat);
 
 	return pvtcfg;
@@ -357,7 +317,6 @@
 {
 	struct skel_config *cfg = obj;
 	ao2_cleanup(cfg->general);
-	ao2_cleanup(cfg->pvts);
 	ao2_cleanup(cfg->cfgs);
 }
 
@@ -375,10 +334,6 @@
 	}
 
 	if (ast_string_field_init(cfg->general, 128)) {
-		goto error;
-	}
-
-	if (!(cfg->pvts = ao2_container_alloc(PVT_BUCKETS, skel_pvt_hash, skel_pvt_cmp))) {
 		goto error;
 	}
 
@@ -424,7 +379,7 @@
 {
 	RAII_VAR(struct skel_config *, cfg, NULL, ao2_cleanup);
 	struct ao2_iterator iter;
-	struct skel_pvt *pvt;
+	struct skel_pvt_config *pcfg;
 	char codec_buf[128];
 
 	switch(cmd) {
@@ -438,26 +393,20 @@
 		return NULL;
 	}
 
-	if (!(cfg = ao2_global_obj_ref(globals)) || !(cfg->cfgs && cfg->pvts)) {
+	if (!(cfg = ao2_global_obj_ref(globals)) || !cfg->cfgs) {
 		return NULL;
 	}
 
 #define SKEL_FORMAT "%-15.15s %-25.25s %-20.20s %-5.5s %-5.5s %-5.5s %-2.2s\n"
 #define SKEL_FORMAT1 "%-15.15s %-25.25s %-20.20s %-5.5s %-5.5s %-5.5s %-2.2zu\n"
 	ast_cli(a->fd, SKEL_FORMAT, "Name", "Description", "Codecs", "ACL", "Bit1", "Bit2", "N");
-	iter = ao2_iterator_init(cfg->pvts, 0);
-	while ((pvt = ao2_iterator_next(&iter))) {
-		RAII_VAR(struct skel_pvt_config *, pcfg, NULL, ao2_cleanup);
-		if (!(pcfg = ao2_find(cfg->cfgs, pvt->name, OBJ_KEY))) {
-			ast_log(LOG_NOTICE, "Could not find config info for %s\n", pvt->name);
-			ao2_ref(pvt, -1);
-			continue;
-		}
+	iter = ao2_iterator_init(cfg->cfgs, 0);
+	while ((pcfg = ao2_iterator_next(&iter))) {
 		/* As an example of non-config-related state remaining between reloads */
-		++pvt->non_config_state;
+		++pcfg->pvt->non_config_state;
 		ast_getformatname_multiple(codec_buf, sizeof(codec_buf) - 1, pcfg->caps);
-		ast_cli(a->fd, SKEL_FORMAT1, pvt->name, pcfg->description, codec_buf, AST_CLI_YESNO(pcfg->permit != NULL), AST_CLI_YESNO(pcfg->bit1), AST_CLI_YESNO(pcfg->bit2), pvt->non_config_state);
-		ao2_ref(pvt, -1);
+		ast_cli(a->fd, SKEL_FORMAT1, pcfg->name, pcfg->description, codec_buf, AST_CLI_YESNO(pcfg->permit != NULL), AST_CLI_YESNO(pcfg->bit1), AST_CLI_YESNO(pcfg->bit2), pcfg->pvt->non_config_state);
+		ao2_ref(pcfg, -1);
 	}
 	ao2_iterator_destroy(&iter);
 #undef SKEL_FORMAT

Modified: team/twilson/config_work/include/asterisk/config_options.h
URL: http://svnview.digium.com/svn/asterisk/team/twilson/config_work/include/asterisk/config_options.h?view=diff&rev=366910&r1=366909&r2=366910
==============================================================================
--- team/twilson/config_work/include/asterisk/config_options.h (original)
+++ team/twilson/config_work/include/asterisk/config_options.h Fri May 18 09:37:35 2012
@@ -57,28 +57,13 @@
  */
 typedef void *(*aco_type_alloc)(const char *category);
 
-/*! \brief Allocate scontainers for a configurable object and its non-config-related state
- * \param newpvts The new private container
- * \param newcfgs The new config container
- * \retval 0 success
- * \retval -1 failure
- */
-typedef int (*aco_type_containers_alloc)(struct ao2_container **newpvts, struct ao2_container **newcfgs);
-
-/*! \brief Find an existing private or create a new one
- * \param category The category associated with the private
- * \retval NULL error
- * \retval non-NULL a referenced ao2 private object
- */
-typedef void *(*aco_type_find_or_create_pvt)(const char *category);
-
 /*! \brief Find a private given a category and container of privates
  * \param container The container to search for the private
  * \param category The category associated with the private
- * \retval NULL error
- * \retval non-NULL a referenced ao2 private object
- */
-typedef void *(*aco_type_find_pvt)(struct ao2_container *newcontainer, const char *category);
+ * \retval 1 config exists in container
+ * \retval 0 config does not exist in container
+ */
+typedef int (*aco_type_cfg_exists)(struct ao2_container *newcontainer, const char *category);
 
 /*! \brief Callback function that is called after a config object is initialized with defaults
  *
@@ -121,9 +106,7 @@
 	aco_type_alloc cfg_alloc; /*!< An allocation function for ao2 object associated with this type */
 
 	/* non-global callbacks */
-	aco_type_containers_alloc containers_alloc;     /*!< A callback function for allocating containers for a config object and its associated private */
-	aco_type_find_or_create_pvt find_or_create_pvt; /*!< A callback function to find an existing private, or create a new one if it doesn't exist */
-	aco_type_find_pvt find_pvt;           /*!< A callback function to find an existing private in a particular container */
+	aco_type_cfg_exists cfg_exists;           /*!< A callback function to find an existing private in a particular container */
 	aco_type_post_cfg_init post_cfg_init; /*!< An optional callback function that is called after defaults are applied, but before config processing */
 	aco_type_prelink prelink;             /*!< An optional callback function that is called after config processing, but before applying changes */
 };

Modified: team/twilson/config_work/main/config_options.c
URL: http://svnview.digium.com/svn/asterisk/team/twilson/config_work/main/config_options.c?view=diff&rev=366910&r1=366909&r2=366910
==============================================================================
--- team/twilson/config_work/main/config_options.c (original)
+++ team/twilson/config_work/main/config_options.c Fri May 18 09:37:35 2012
@@ -248,7 +248,6 @@
 }
 
 static int process_category(struct ast_config *cfg, struct aco_info *info, const char *cat, int preload) {
-	RAII_VAR(void *, tmppvt, NULL, ao2_cleanup);
 	RAII_VAR(void *, tmpcfg, NULL, ao2_cleanup);
 	struct aco_type *obj;
 
@@ -276,23 +275,16 @@
 			return -1;
 		}
 	} else if (obj->type == ACO_PRIVATE_OBJ) {
-		void **field = info->internal->pending + obj->pvt_offset;
+		void **field = info->internal->pending + obj->cfg_offset;
 		/* If we've already linked a private for cat in newpvts, don't add a second one with the same name */
 		if (*field) {
-			if ((tmppvt = obj->find_pvt(*field, cat))) {
+			if ((obj->cfg_exists(*field, cat))) {
 				ast_log(LOG_ERROR, "In %s: Multiple definitions of %s!\n", info->filename, cat);
 				return -1;
 			}
-
-			/* Get a ref to the existing private, or create a new one */
-			if (!(tmppvt = obj->find_or_create_pvt(cat))) {
-				/* Since we will be replacing the whole private container, bail out on errors instead of just
-				 * skipping privates with config errors */
-				ast_log(LOG_ERROR, "In %s: Could not create private object for %s\n", info->filename, cat);
-				return -1;
-			}
-		}
-
+		}
+
+		/* allocates a private if necessary */
 		if (!(tmpcfg = obj->cfg_alloc(cat))) {
 			ast_log(LOG_ERROR, "In %s: Could not create private config object for %s\n", info->filename, cat);
 			return -1;
@@ -319,11 +311,6 @@
 		}
 
 		/* We have a valid pvt/cfg, link 'em */
-		if (tmppvt && !ao2_link(*field, tmppvt)) {
-			ast_log(LOG_ERROR, "In %s: Linking private for %s failed\n", info->filename, cat);
-			return -1;
-		}
-		field = info->internal->pending + obj->cfg_offset;
 		if (!ao2_link(*field, tmpcfg)) {
 			ast_log(LOG_ERROR, "In %s: Linking config for %s failed\n", info->filename, cat);
 			return -1;
@@ -373,7 +360,6 @@
 	return res;
 
 error:
-	ast_config_destroy(cfg);
 	ao2_ref(info->internal->pending, -1);
 	return -1;
 }




More information about the asterisk-commits mailing list