[Asterisk-code-review] res/ari/config.c: Optimize conf alloc() object init. (asterisk[master])

[Matt Jordan]

Matt Jordan has submitted this change and it was merged.

Change subject: res/ari/config.c: Optimize conf_alloc() object init.
......................................................................


res/ari/config.c: Optimize conf_alloc() object init.

* Now conf_alloc() has more off nominal error checking.

* Eliminated RAII_VAR() use in conf_alloc().

* Eliminated a dubius shortcut when destroying cfg->general in
conf_destructor() that would cause a crash if cfg->general failed to get
allocated.

* Add some ACO registration section comments.

Change-Id: Ia40c2b1b2d0777d641605118ae019c5a73865e1a
---
M res/ari/config.c
1 file changed, 19 insertions(+), 12 deletions(-)

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



diff --git a/res/ari/config.c b/res/ari/config.c
index 6d2a679..ba06fe9 100644
--- a/res/ari/config.c
+++ b/res/ari/config.c
@@ -155,12 +155,17 @@
 
 static struct aco_type *user[] = ACO_TYPES(&user_option);
 
+static void conf_general_dtor(void *obj)
+{
+	struct ast_ari_conf_general *general = obj;
+
+	ast_string_field_free_memory(general);
+}
+
 /*! \brief \ref ast_ari_conf destructor. */
 static void conf_destructor(void *obj)
 {
 	struct ast_ari_conf *cfg = obj;
-
-	ast_string_field_free_memory(cfg->general);
 
 	ao2_cleanup(cfg->general);
 	ao2_cleanup(cfg->users);
@@ -169,7 +174,7 @@
 /*! \brief Allocate an \ref ast_ari_conf for config parsing */
 static void *conf_alloc(void)
 {
-	RAII_VAR(struct ast_ari_conf *, cfg, NULL, ao2_cleanup);
+	struct ast_ari_conf *cfg;
 
 	cfg = ao2_alloc_options(sizeof(*cfg), conf_destructor,
 		AO2_ALLOC_OPT_LOCK_NOLOCK);
@@ -177,20 +182,20 @@
 		return NULL;
 	}
 
-	cfg->general = ao2_alloc_options(sizeof(*cfg->general), NULL,
+	cfg->general = ao2_alloc_options(sizeof(*cfg->general), conf_general_dtor,
 		AO2_ALLOC_OPT_LOCK_NOLOCK);
-	if (!cfg->general) {
-		return NULL;
-	}
-	if (ast_string_field_init(cfg->general, 64)) {
-		return NULL;
-	}
-	aco_set_defaults(&general_option, "general", cfg->general);
 
 	cfg->users = ao2_container_alloc_rbtree(AO2_ALLOC_OPT_LOCK_NOLOCK,
 		AO2_CONTAINER_ALLOC_OPT_DUPS_REPLACE, user_sort_cmp, NULL);
 
-	ao2_ref(cfg, +1);
+	if (!cfg->users
+		|| !cfg->general
+		|| ast_string_field_init(cfg->general, 64)
+		|| aco_set_defaults(&general_option, "general", cfg->general)) {
+		ao2_ref(cfg, -1);
+		return NULL;
+	}
+
 	return cfg;
 }
 
@@ -308,6 +313,7 @@
 		return -1;
 	}
 
+	/* ARI general category options */
 	aco_option_register(&cfg_info, "enabled", ACO_EXACT, general_options,
 		"yes", OPT_BOOL_T, 1,
 		FLDSET(struct ast_ari_conf_general, enabled));
@@ -324,6 +330,7 @@
 		AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT_STR, OPT_INT_T, PARSE_IN_RANGE,
 		FLDSET(struct ast_ari_conf_general, write_timeout), 1, INT_MAX);
 
+	/* ARI type=user category options */
 	aco_option_register(&cfg_info, "type", ACO_EXACT, user, NULL,
 		OPT_NOOP_T, 0, 0);
 	aco_option_register(&cfg_info, "read_only", ACO_EXACT, user,

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia40c2b1b2d0777d641605118ae019c5a73865e1a
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>