Fix reloading an unchanged file with the Config Options API
authorTerry Wilson <twilson@digium.com>
Thu, 7 Jun 2012 20:32:07 +0000 (20:32 +0000)
committerTerry Wilson <twilson@digium.com>
Thu, 7 Jun 2012 20:32:07 +0000 (20:32 +0000)
Adding multiple file support broke reloading an unchanged file. This
adds an enum for return values for the aco_process_* functions and
ensures that the config is not applied if res is not ACO_PROCESS_OK.

Review: https://reviewboard.asterisk.org/r/1979/

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@368673 65c4cc65-6c06-0410-ace0-fbb531ad65f3

apps/app_skel.c
include/asterisk/config_options.h
main/config_options.c
main/udptl.c
tests/test_config.c

index 02f09d7..ca96fa0 100644 (file)
@@ -633,7 +633,7 @@ static struct ast_cli_entry skel_cli[] = {
 
 static int reload_module(void)
 {
-       if (aco_process_config(&cfg_info, 1)) {
+       if (aco_process_config(&cfg_info, 1) == ACO_PROCESS_ERROR) {
                return AST_MODULE_LOAD_DECLINE;
        }
 
@@ -673,7 +673,7 @@ static int load_module(void)
        aco_option_register(&cfg_info, "max_number", ACO_EXACT, level_options, NULL, OPT_UINT_T, 0, FLDSET(struct skel_level, max_num));
        aco_option_register(&cfg_info, "max_guesses", ACO_EXACT, level_options, NULL, OPT_UINT_T, 1, FLDSET(struct skel_level, max_guesses));
 
-       if (aco_process_config(&cfg_info, 0)) {
+       if (aco_process_config(&cfg_info, 0) == ACO_PROCESS_ERROR) {
                goto error;
        }
 
index 029434a..b697ecb 100644 (file)
@@ -386,15 +386,24 @@ typedef int (*aco_option_handler)(const struct aco_option *opt, struct ast_varia
 /*! \brief Allocate a container to hold config options */
 struct ao2_container *aco_option_container_alloc(void);
 
+/*! \brief Return values for the aco_process functions
+ */
+enum aco_process_status {
+       ACO_PROCESS_OK,        /*!< \brief The config was processed and applied */
+       ACO_PROCESS_UNCHANGED, /*!< \brief The config had not been edited and no changes applied */
+       ACO_PROCESS_ERROR,     /*!< \brief Their was an error and no changes were applied */
+};
+
 /*! \brief Process a config info via the options registered with an aco_info
  *
  * \param info The config_options_info to be used for handling the config
  * \param reload Whether or not this is a reload
  *
- * \retval 0 Success
- * \retval -1 Failure
+ * \retval ACO_PROCESS_OK Success
+ * \retval ACO_PROCESS_ERROR Failure
+ * \retval ACO_PROCESS_UNCHANGED No change due to unedited config file
  */
-int aco_process_config(struct aco_info *info, int reload);
+enum aco_process_status aco_process_config(struct aco_info *info, int reload);
 
 /*! \brief Process config info from an ast_config via options registered with an aco_info
  *
@@ -403,10 +412,10 @@ int aco_process_config(struct aco_info *info, int reload);
  * \param cfg A pointer to a loaded ast_config to parse
  * \param reload Whether or not this is a reload
  *
- * \retval 0 Success
- * \retval -1 Failure
+ * \retval ACO_PROCESS_OK Success
+ * \retval ACO_PROCESS_ERROR Failure
  */
-int aco_process_ast_config(struct aco_info *info, struct aco_file *file, struct ast_config *cfg);
+enum aco_process_status aco_process_ast_config(struct aco_info *info, struct aco_file *file, struct ast_config *cfg);
 
 /*! \brief Parse each option defined in a config category
  * \param type The aco_type with the options for parsing
index de89d8d..c8fb4ff 100644 (file)
@@ -368,7 +368,7 @@ static int apply_config(struct aco_info *info)
        return 0;
 }
 
-static int internal_process_ast_config(struct aco_info *info, struct aco_file *file, struct ast_config *cfg)
+static enum aco_process_status internal_process_ast_config(struct aco_info *info, struct aco_file *file, struct ast_config *cfg)
 {
        const char *cat = NULL;
 
@@ -376,20 +376,20 @@ static int internal_process_ast_config(struct aco_info *info, struct aco_file *f
                int i;
                for (i = 0; !ast_strlen_zero(file->preload[i]); i++) {
                        if (process_category(cfg, info, file, file->preload[i], 1)) {
-                               return -1;
+                               return ACO_PROCESS_ERROR;
                        }
                }
        }
 
        while ((cat = ast_category_browse(cfg, cat))) {
                if (process_category(cfg, info, file, cat, 0)) {
-                       return -1;
+                       return ACO_PROCESS_ERROR;
                }
        }
-       return 0;
+       return ACO_PROCESS_OK;
 }
 
-int aco_process_ast_config(struct aco_info *info, struct aco_file *file, struct ast_config *cfg)
+enum aco_process_status aco_process_ast_config(struct aco_info *info, struct aco_file *file, struct ast_config *cfg)
 {
        if (!(info->internal->pending = info->snapshot_alloc())) {
                ast_log(LOG_ERROR, "In %s: Could not allocate temporary objects\n", file->filename);
@@ -409,46 +409,46 @@ int aco_process_ast_config(struct aco_info *info, struct aco_file *file, struct
        };
 
        ao2_cleanup(info->internal->pending);
-       return 0;
+       return ACO_PROCESS_OK;
 
 error:
        ao2_cleanup(info->internal->pending);
-       return -1;
+       return ACO_PROCESS_ERROR;
 }
 
-int aco_process_config(struct aco_info *info, int reload)
+enum aco_process_status aco_process_config(struct aco_info *info, int reload)
 {
        struct ast_config *cfg;
        struct ast_flags cfg_flags = { reload ? CONFIG_FLAG_FILEUNCHANGED : 0, };
-       int res = 0, x = 0;
+       int res = ACO_PROCESS_OK, x = 0;
        struct aco_file *file;
 
        if (!(info->files[0])) {
                ast_log(LOG_ERROR, "No filename given, cannot proceed!\n");
-               return -1;
+               return ACO_PROCESS_ERROR;
        }
 
        if (!(info->internal->pending = info->snapshot_alloc())) {
                ast_log(LOG_ERROR, "In %s: Could not allocate temporary objects\n", info->module);
-               return -1;
+               return ACO_PROCESS_ERROR;
        }
 
-       while (!res && (file = info->files[x++])) {
+       while (res != ACO_PROCESS_ERROR && (file = info->files[x++])) {
                if (!(cfg = ast_config_load(file->filename, cfg_flags))) {
                        ast_log(LOG_ERROR, "Unable to load config file '%s'\n", file->filename);
-                       res = -1;
+                       res = ACO_PROCESS_ERROR;
                        break;
                } else if (cfg == CONFIG_STATUS_FILEUNCHANGED) {
                        ast_debug(1, "%s was unchanged\n", file->filename);
-                       res = 0;
+                       res = ACO_PROCESS_UNCHANGED;
                        continue;
                } else if (cfg == CONFIG_STATUS_FILEINVALID) {
                        ast_log(LOG_ERROR, "Contents of %s are invalid and cannot be parsed\n", file->filename);
-                       res = -1;
+                       res = ACO_PROCESS_ERROR;
                        break;
                } else if (cfg == CONFIG_STATUS_FILEMISSING) {
                        ast_log(LOG_ERROR, "%s is missing! Cannot load %s\n", file->filename, info->module);
-                       res = -1;
+                       res = ACO_PROCESS_ERROR;
                        break;
                }
 
@@ -456,10 +456,21 @@ int aco_process_config(struct aco_info *info, int reload)
                ast_config_destroy(cfg);
        }
 
-       if (res || (res = ((info->pre_apply_config && info->pre_apply_config()) || apply_config(info)))) {
-               ;
-       };
+       if (res != ACO_PROCESS_OK) {
+          goto end;
+       }
+
+       if (info->pre_apply_config && (info->pre_apply_config()))  {
+               res = ACO_PROCESS_ERROR;
+               goto end;
+       }
+
+       if (apply_config(info)) {
+               res = ACO_PROCESS_ERROR;
+               goto end;
+       }
 
+end:
        ao2_cleanup(info->internal->pending);
        return res;
 }
index a550ff1..e31d9f8 100644 (file)
@@ -1411,7 +1411,7 @@ static int removed_options_handler(const struct aco_option *opt, struct ast_vari
 
 static void __ast_udptl_reload(int reload)
 {
-       if (aco_process_config(&cfg_info, reload)) {
+       if (aco_process_config(&cfg_info, reload) == ACO_PROCESS_ERROR) {
                ast_log(LOG_WARNING, "Could not reload udptl config\n");
        }
 }
index 154b824..4d92c79 100644 (file)
@@ -811,7 +811,7 @@ AST_TEST_DEFINE(config_options_test)
        aco_option_register(&cfg_info, "stropt", ACO_EXACT, config_test_conf.types, STR_DEFAULT, OPT_STRINGFIELD_T, 0, STRFLDSET(struct test_item, stropt));
        aco_option_register_custom(&cfg_info, "customopt", ACO_EXACT, config_test_conf.types, CUSTOM_DEFAULT, customopt_handler, 0);
 
-       if (aco_process_config(&cfg_info, 0)) {
+       if (aco_process_config(&cfg_info, 0) == ACO_PROCESS_ERROR) {
                ast_test_status_update(test, "Could not parse config\n");
                return AST_TEST_FAIL;
        }