Merged revisions 334297 via svnmerge from
authorRichard Mudgett <rmudgett@digium.com>
Fri, 2 Sep 2011 17:19:17 +0000 (17:19 +0000)
committerRichard Mudgett <rmudgett@digium.com>
Fri, 2 Sep 2011 17:19:17 +0000 (17:19 +0000)
https://origsvn.digium.com/svn/asterisk/branches/10

................
  r334297 | rmudgett | 2011-09-02 12:15:08 -0500 (Fri, 02 Sep 2011) | 46 lines

  Merged revisions 334296 via svnmerge from
  https://origsvn.digium.com/svn/asterisk/branches/1.8

  ........
    r334296 | rmudgett | 2011-09-02 12:10:58 -0500 (Fri, 02 Sep 2011) | 39 lines

    Fix potential memory allocation failure crashes in config.c.

    * Added required checks to the returned memory allocation pointers to
    prevent crashes.

    * Made ast_include_rename() create a replacement ast_variable list node if
    the new filename is longer than the available space.  Fixes potential
    crash and memory leak.

    * Factored out ast_variable_move() from ast_variable_update() so
    ast_include_rename() can also use it when creating a replacement
    ast_variable list node.

    * Made the filename stuffed at the end of the struct a minimum allocated
    size in ast_variable_new() in case ast_include_rename() changes the stored
    filename.

    * Constify struct char pointers pointing to strings stuffed at the end of
    the struct for: ast_variable, cache_file_mtime, and ast_config_map.

    * Factored out cfmtime_new() to remove inlined code and allow some struct
    pointers to become const.

    * Removed the list lock from struct cache_file_mtime that was never used.

    * Added doxygen comments to several structure elements and better
    documented what strings are stuffed at the struct end char array.

    * Reworked ast_config_text_file_save() and set_fn() to handle allocation
    failure of the include file scratch pad object tracking blank lines.

    * Made ast_config_text_file_save() fn[] declared with PATH_MAX to ensure
    it is long enough for any filename with path.  Also reduced the number of
    container fileset buckets from a rediculus 180,000 to 1023.

    JIRA AST-618

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

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

include/asterisk/config.h
main/config.c

index f7279f1..56401bf 100644 (file)
@@ -73,11 +73,16 @@ typedef enum {
 
 /*! \brief Structure for variables, used for configurations and for channel variables */
 struct ast_variable {
+       /*! Variable name.  Stored in stuff[] at struct end. */
        const char *name;
+       /*! Variable value.  Stored in stuff[] at struct end. */
        const char *value;
+
+       /*! Next node in the list. */
        struct ast_variable *next;
 
-       char *file;
+       /*! Filename where variable found.  Stored in stuff[] at struct end. */
+       const char *file;
 
        int lineno;
        int object;             /*!< 0 for variable, 1 for object */
@@ -85,6 +90,10 @@ struct ast_variable {
        struct ast_comment *precomments;
        struct ast_comment *sameline;
        struct ast_comment *trailing; /*!< the last object in the list will get assigned any trailing comments when EOF is hit */
+       /*!
+        * \brief Contents of file, name, and value in that order stuffed here.
+        * \note File must be stuffed before name because of ast_include_rename().
+        */
        char stuff[0];
 };
 
index 43a109a..e432de3 100644 (file)
@@ -55,12 +55,20 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 #define COMMENT_META ';'
 #define COMMENT_TAG '-'
 
+/*!
+ * Define the minimum filename space to reserve for each
+ * ast_variable in case the filename is renamed later by
+ * ast_include_rename().
+ */
+#define MIN_VARIABLE_FNAME_SPACE       40
+
 static char *extconfig_conf = "extconfig.conf";
 
 
 /*! \brief Structure to keep comments for rewriting configuration files */
 struct ast_comment {
        struct ast_comment *next;
+       /*! Comment body allocated after struct. */
        char cmt[0];
 };
 
@@ -72,13 +80,17 @@ struct cache_file_include {
 
 struct cache_file_mtime {
        AST_LIST_ENTRY(cache_file_mtime) list;
-       AST_LIST_HEAD(includes, cache_file_include) includes;
+       AST_LIST_HEAD_NOLOCK(includes, cache_file_include) includes;
        unsigned int has_exec:1;
        time_t mtime;
-       char *who_asked;
+
+       /*! String stuffed in filename[] after the filename string. */
+       const char *who_asked;
+       /*! Filename and who_asked stuffed after it. */
        char filename[0];
 };
 
+/*! Cached file mtime list. */
 static AST_LIST_HEAD_STATIC(cfmtime_head, cache_file_mtime);
 
 static int init_appendbuf(void *data)
@@ -164,10 +176,15 @@ static int hashtab_compare_strings(void *a, void *b, int flags)
 static struct ast_config_map {
        struct ast_config_map *next;
        int priority;
-       char *name;
-       char *driver;
-       char *database;
-       char *table;
+       /*! Stored in stuff[] at struct end. */
+       const char *name;
+       /*! Stored in stuff[] at struct end. */
+       const char *driver;
+       /*! Stored in stuff[] at struct end. */
+       const char *database;
+       /*! Stored in stuff[] at struct end. */
+       const char *table;
+       /*! Contents of name, driver, database, and table in that order stuffed here. */
        char stuff[0];
 } *config_maps = NULL;
 
@@ -186,19 +203,28 @@ struct ast_category {
        char name[80];
        int ignored;                    /*!< do not let user of the config see this category -- set by (!) after the category decl; a template */
        int include_level;
-       char *file;                /*!< the file name from whence this declaration was read */
+       /*!
+        * \brief The file name from whence this declaration was read
+        * \note Will never be NULL
+        */
+       char *file;
        int lineno;
        AST_LIST_HEAD_NOLOCK(template_instance_list, ast_category_template_instance) template_instances;
        struct ast_comment *precomments;
        struct ast_comment *sameline;
        struct ast_comment *trailing; /*!< the last object in the list will get assigned any trailing comments when EOF is hit */
+       /*! First category variable in the list. */
        struct ast_variable *root;
+       /*! Last category variable in the list. */
        struct ast_variable *last;
+       /*! Next node in the list. */
        struct ast_category *next;
 };
 
 struct ast_config {
+       /*! First config category in the list. */
        struct ast_category *root;
+       /*! Last config category in the list. */
        struct ast_category *last;
        struct ast_category *current;
        struct ast_category *last_browse;     /*!< used to cache the last category supplied via category_browse */
@@ -208,43 +234,89 @@ struct ast_config {
 };
 
 struct ast_config_include {
-       char *include_location_file;     /*!< file name in which the include occurs */
+       /*!
+        * \brief file name in which the include occurs
+        * \note Will never be NULL
+        */
+       char *include_location_file;
        int  include_location_lineno;    /*!< lineno where include occurred */
-       int  exec;                       /*!< set to non-zero if itsa #exec statement */
-       char *exec_file;                 /*!< if it's an exec, you'll have both the /var/tmp to read, and the original script */
-       char *included_file;             /*!< file name included */
+       int  exec;                       /*!< set to non-zero if its a #exec statement */
+       /*!
+        * \brief if it's an exec, you'll have both the /var/tmp to read, and the original script
+        * \note Will never be NULL if exec is non-zero
+        */
+       char *exec_file;
+       /*!
+        * \brief file name included
+        * \note Will never be NULL
+        */
+       char *included_file;
        int inclusion_count;             /*!< if the file is included more than once, a running count thereof -- but, worry not,
                                              we explode the instances and will include those-- so all entries will be unique */
        int output;                      /*!< a flag to indicate if the inclusion has been output */
        struct ast_config_include *next; /*!< ptr to next inclusion in the list */
 };
 
+static void ast_variable_destroy(struct ast_variable *doomed);
+static void ast_includes_destroy(struct ast_config_include *incls);
+
 #ifdef MALLOC_DEBUG
-struct ast_variable *_ast_variable_new(const char *name, const char *value, const char *filename, const char *file, const char *func, int lineno) 
+struct ast_variable *_ast_variable_new(const char *name, const char *value, const char *filename, const char *file, const char *func, int lineno)
 #else
-struct ast_variable *ast_variable_new(const char *name, const char *value, const char *filename) 
+struct ast_variable *ast_variable_new(const char *name, const char *value, const char *filename)
 #endif
 {
        struct ast_variable *variable;
-       int name_len = strlen(name) + 1;        
-       int val_len = strlen(value) + 1;        
-       int fn_len = strlen(filename) + 1;      
+       int name_len = strlen(name) + 1;
+       int val_len = strlen(value) + 1;
+       int fn_len = strlen(filename) + 1;
 
+       /* Ensure a minimum length in case the filename is changed later. */
+       if (fn_len < MIN_VARIABLE_FNAME_SPACE) {
+               fn_len = MIN_VARIABLE_FNAME_SPACE;
+       }
+
+       if (
 #ifdef MALLOC_DEBUG
-       if ((variable = __ast_calloc(1, name_len + val_len + fn_len + sizeof(*variable), file, lineno, func))) {
+               (variable = __ast_calloc(1, fn_len + name_len + val_len + sizeof(*variable), file, lineno, func))
 #else
-       if ((variable = ast_calloc(1, name_len + val_len + fn_len + sizeof(*variable)))) {
+               (variable = ast_calloc(1, fn_len + name_len + val_len + sizeof(*variable)))
 #endif
+               ) {
                char *dst = variable->stuff;    /* writable space starts here */
+
+               /* Put file first so ast_include_rename() can calculate space available. */
+               variable->file = strcpy(dst, filename);
+               dst += fn_len;
                variable->name = strcpy(dst, name);
                dst += name_len;
                variable->value = strcpy(dst, value);
-               dst += val_len;
-               variable->file = strcpy(dst, filename);
        }
        return variable;
 }
 
+/*!
+ * \internal
+ * \brief Move the contents from the source to the destination variable.
+ *
+ * \param dst_var Destination variable node
+ * \param src_var Source variable node
+ *
+ * \return Nothing
+ */
+static void ast_variable_move(struct ast_variable *dst_var, struct ast_variable *src_var)
+{
+       dst_var->lineno = src_var->lineno;
+       dst_var->object = src_var->object;
+       dst_var->blanklines = src_var->blanklines;
+       dst_var->precomments = src_var->precomments;
+       src_var->precomments = NULL;
+       dst_var->sameline = src_var->sameline;
+       src_var->sameline = NULL;
+       dst_var->trailing = src_var->trailing;
+       src_var->trailing = NULL;
+}
+
 struct ast_config_include *ast_include_new(struct ast_config *conf, const char *from_file, const char *included_file, int is_exec, const char *exec_file, int from_lineno, char *real_included_file_name, int real_included_file_name_size)
 {
        /* a file should be included ONCE. Otherwise, if one of the instances is changed,
@@ -265,6 +337,9 @@ struct ast_config_include *ast_include_new(struct ast_config *conf, const char *
                *real_included_file_name = 0;
        
        inc = ast_calloc(1,sizeof(struct ast_config_include));
+       if (!inc) {
+               return NULL;
+       }
        inc->include_location_file = ast_strdup(from_file);
        inc->include_location_lineno = from_lineno;
        if (!ast_strlen_zero(real_included_file_name))
@@ -275,7 +350,14 @@ struct ast_config_include *ast_include_new(struct ast_config *conf, const char *
        inc->exec = is_exec;
        if (is_exec)
                inc->exec_file = ast_strdup(exec_file);
-       
+
+       if (!inc->include_location_file
+               || !inc->included_file
+               || (is_exec && !inc->exec_file)) {
+               ast_includes_destroy(inc);
+               return NULL;
+       }
+
        /* attach this new struct to the conf struct */
        inc->next = conf->includes;
        conf->includes = inc;
@@ -287,8 +369,8 @@ void ast_include_rename(struct ast_config *conf, const char *from_file, const ch
 {
        struct ast_config_include *incl;
        struct ast_category *cat;
-       struct ast_variable *v;
-       
+       char *str;
+
        int from_len = strlen(from_file);
        int to_len = strlen(to_file);
        
@@ -309,29 +391,68 @@ void ast_include_rename(struct ast_config *conf, const char *from_file, const ch
                        if (from_len >= to_len)
                                strcpy(incl->include_location_file, to_file);
                        else {
-                               free(incl->include_location_file);
-                               incl->include_location_file = strdup(to_file);
+                               /* Keep the old filename if the allocation fails. */
+                               str = ast_strdup(to_file);
+                               if (str) {
+                                       ast_free(incl->include_location_file);
+                                       incl->include_location_file = str;
+                               }
                        }
                }
        }
        for (cat = conf->root; cat; cat = cat->next) {
+               struct ast_variable **prev;
+               struct ast_variable *v;
+               struct ast_variable *new_var;
+
                if (strcmp(cat->file,from_file) == 0) {
                        if (from_len >= to_len)
                                strcpy(cat->file, to_file);
                        else {
-                               free(cat->file);
-                               cat->file = strdup(to_file);
+                               /* Keep the old filename if the allocation fails. */
+                               str = ast_strdup(to_file);
+                               if (str) {
+                                       ast_free(cat->file);
+                                       cat->file = str;
+                               }
                        }
                }
-               for (v = cat->root; v; v = v->next) {
-                       if (strcmp(v->file,from_file) == 0) {
-                               if (from_len >= to_len)
-                                       strcpy(v->file, to_file);
-                               else {
-                                       free(v->file);
-                                       v->file = strdup(to_file);
-                               }
+               for (prev = &cat->root, v = cat->root; v; prev = &v->next, v = v->next) {
+                       if (strcmp(v->file, from_file)) {
+                               continue;
+                       }
+
+                       /*
+                        * Calculate actual space available.  The file string is
+                        * intentionally stuffed before the name string just so we can
+                        * do this.
+                        */
+                       if (to_len < v->name - v->file) {
+                               /* The new name will fit in the available space. */
+                               str = (char *) v->file;/* Stupid compiler complains about discarding qualifiers even though I used a cast. */
+                               strcpy(str, to_file);/* SAFE */
+                               continue;
+                       }
+
+                       /* Keep the old filename if the allocation fails. */
+                       new_var = ast_variable_new(v->name, v->value, to_file);
+                       if (!new_var) {
+                               continue;
                        }
+
+                       /* Move items from the old list node to the replacement node. */
+                       ast_variable_move(new_var, v);
+
+                       /* Replace the old node in the list with the new node. */
+                       new_var->next = v->next;
+                       if (cat->last == v) {
+                               cat->last = new_var;
+                       }
+                       *prev = new_var;
+
+                       ast_variable_destroy(v);
+
+                       v = new_var;
                }
        }
 }
@@ -491,9 +612,16 @@ struct ast_category *ast_category_new(const char *name, const char *in_file, int
 {
        struct ast_category *category;
 
-       if ((category = ast_calloc(1, sizeof(*category))))
-               ast_copy_string(category->name, name, sizeof(category->name));
-       category->file = strdup(in_file);
+       category = ast_calloc(1, sizeof(*category));
+       if (!category) {
+               return NULL;
+       }
+       category->file = ast_strdup(in_file);
+       if (!category->file) {
+               ast_category_destroy(category);
+               return NULL;
+       }
+       ast_copy_string(category->name, name, sizeof(category->name));
        category->lineno = lineno; /* if you don't know the lineno, set it to 999999 or something real big */
        return category;
 }
@@ -562,20 +690,19 @@ static void ast_destroy_template_list(struct ast_category *cat)
        struct ast_category_template_instance *x;
 
        while ((x = AST_LIST_REMOVE_HEAD(&cat->template_instances, next)))
-               free(x);
+               ast_free(x);
 }
 
 void ast_category_destroy(struct ast_category *cat)
 {
        ast_variables_destroy(cat->root);
-       if (cat->file) {
-               free(cat->file);
-               cat->file = 0;
-       }
+       cat->root = NULL;
+       cat->last = NULL;
        ast_comment_destroy(&cat->precomments);
        ast_comment_destroy(&cat->sameline);
        ast_comment_destroy(&cat->trailing);
        ast_destroy_template_list(cat);
+       ast_free(cat->file);
        ast_free(cat);
 }
 
@@ -585,13 +712,10 @@ static void ast_includes_destroy(struct ast_config_include *incls)
        
        for (incl=incls; incl; incl = inclnext) {
                inclnext = incl->next;
-               if (incl->include_location_file)
-                       free(incl->include_location_file);
-               if (incl->exec_file)
-                       free(incl->exec_file);
-               if (incl->included_file)
-                       free(incl->included_file);
-               free(incl);
+               ast_free(incl->include_location_file);
+               ast_free(incl->exec_file);
+               ast_free(incl->included_file);
+               ast_free(incl);
        }
 }
 
@@ -682,8 +806,12 @@ void ast_category_rename(struct ast_category *cat, const char *name)
 static void inherit_category(struct ast_category *new, const struct ast_category *base)
 {
        struct ast_variable *var;
-       struct ast_category_template_instance *x = ast_calloc(1,sizeof(struct ast_category_template_instance));
+       struct ast_category_template_instance *x;
 
+       x = ast_calloc(1, sizeof(*x));
+       if (!x) {
+               return;
+       }
        strcpy(x->name, base->name);
        x->inst = base;
        AST_LIST_INSERT_TAIL(&new->template_instances, x, next);
@@ -757,17 +885,12 @@ int ast_variable_update(struct ast_category *category, const char *variable,
 
                if (!(newer = ast_variable_new(variable, value, cur->file)))
                        return -1;
-       
-               newer->next = cur->next;
-               newer->object = cur->object || object;
 
-               /* Preserve everything */
-               newer->lineno = cur->lineno;
-               newer->blanklines = cur->blanklines;
-               newer->precomments = cur->precomments; cur->precomments = NULL;
-               newer->sameline = cur->sameline; cur->sameline = NULL;
-               newer->trailing = cur->trailing; cur->trailing = NULL;
+               ast_variable_move(newer, cur);
+               newer->object = newer->object || object;
 
+               /* Replace the old node in the list with the new node. */
+               newer->next = cur->next;
                if (prev)
                        prev->next = newer;
                else
@@ -775,8 +898,7 @@ int ast_variable_update(struct ast_category *category, const char *variable,
                if (category->last == cur)
                        category->last = newer;
 
-               cur->next = NULL;
-               ast_variables_destroy(cur);
+               ast_variable_destroy(cur);
 
                return 0;
        }
@@ -875,6 +997,34 @@ void ast_config_set_current_category(struct ast_config *cfg, const struct ast_ca
        cfg->current = (struct ast_category *) cat;
 }
 
+/*!
+ * \internal
+ * \brief Create a new cfmtime list node.
+ *
+ * \param filename Config filename caching.
+ * \param who_asked Who wanted to know.
+ *
+ * \retval cfmtime New node on success.
+ * \retval NULL on error.
+ */
+static struct cache_file_mtime *cfmtime_new(const char *filename, const char *who_asked)
+{
+       struct cache_file_mtime *cfmtime;
+       char *dst;
+
+       cfmtime = ast_calloc(1,
+               sizeof(*cfmtime) + strlen(filename) + 1 + strlen(who_asked) + 1);
+       if (!cfmtime) {
+               return NULL;
+       }
+       dst = cfmtime->filename;        /* writable space starts here */
+       strcpy(dst, filename);
+       dst += strlen(dst) + 1;
+       cfmtime->who_asked = strcpy(dst, who_asked);
+
+       return cfmtime;
+}
+
 enum config_cache_attribute_enum {
        ATTRIBUTE_INCLUDE = 0,
        ATTRIBUTE_EXEC = 1,
@@ -893,15 +1043,11 @@ static void config_cache_attribute(const char *configfile, enum config_cache_att
                        break;
        }
        if (!cfmtime) {
-               cfmtime = ast_calloc(1, sizeof(*cfmtime) + strlen(configfile) + 1 + strlen(who_asked) + 1);
+               cfmtime = cfmtime_new(configfile, who_asked);
                if (!cfmtime) {
                        AST_LIST_UNLOCK(&cfmtime_head);
                        return;
                }
-               AST_LIST_HEAD_INIT(&cfmtime->includes);
-               strcpy(cfmtime->filename, configfile);
-               cfmtime->who_asked = cfmtime->filename + strlen(configfile) + 1;
-               strcpy(cfmtime->who_asked, who_asked);
                /* Note that the file mtime is initialized to 0, i.e. 1970 */
                AST_LIST_INSERT_SORTALPHA(&cfmtime_head, cfmtime, list, filename);
        }
@@ -1226,8 +1372,7 @@ static struct ast_config *config_text_file_load(const char *database, const char
                if (comment_buffer)
                        lline_buffer = ast_str_create(CB_SIZE);
                if (!lline_buffer) {
-                       if (comment_buffer)
-                               ast_free(comment_buffer);
+                       ast_free(comment_buffer);
                        ast_log(LOG_ERROR, "Failed to initialize the comment buffer!\n");
                        return NULL;
                }
@@ -1272,13 +1417,9 @@ static struct ast_config *config_text_file_load(const char *database, const char
                                        break;
                        }
                        if (!cfmtime) {
-                               cfmtime = ast_calloc(1, sizeof(*cfmtime) + strlen(fn) + 1 + strlen(who_asked) + 1);
+                               cfmtime = cfmtime_new(fn, who_asked);
                                if (!cfmtime)
                                        continue;
-                               AST_LIST_HEAD_INIT(&cfmtime->includes);
-                               strcpy(cfmtime->filename, fn);
-                               cfmtime->who_asked = cfmtime->filename + strlen(fn) + 1;
-                               strcpy(cfmtime->who_asked, who_asked);
                                /* Note that the file mtime is initialized to 0, i.e. 1970 */
                                AST_LIST_INSERT_SORTALPHA(&cfmtime_head, cfmtime, list, filename);
                        }
@@ -1327,8 +1468,11 @@ static struct ast_config *config_text_file_load(const char *database, const char
                        AST_LIST_UNLOCK(&cfmtime_head);
 
                /* If cfg is NULL, then we just want an answer */
-               if (cfg == NULL)
+               if (cfg == NULL) {
+                       ast_free(comment_buffer);
+                       ast_free(lline_buffer);
                        return NULL;
+               }
 
                if (cfmtime)
                        cfmtime->mtime = statbuf.st_mtime;
@@ -1470,10 +1614,8 @@ static struct ast_config *config_text_file_load(const char *database, const char
 #endif
 
        if (cfg && cfg != CONFIG_STATUS_FILEUNCHANGED && cfg != CONFIG_STATUS_FILEINVALID && cfg->include_level == 1 && ast_test_flag(&flags, CONFIG_FLAG_WITHCOMMENTS)) {
-               if (comment_buffer)
-                       ast_free(comment_buffer);
-               if (lline_buffer)
-                       ast_free(lline_buffer);
+               ast_free(comment_buffer);
+               ast_free(lline_buffer);
                comment_buffer = NULL;
                lline_buffer = NULL;
        }
@@ -1524,38 +1666,52 @@ static void gen_header(FILE *f1, const char *configfile, const char *fn, const c
        fprintf(f1, ";!\n");
 }
 
-static void   inclfile_destroy(void *obj)
+static void inclfile_destroy(void *obj)
 {
        const struct inclfile *o = obj;
 
-       if (o->fname)
-               free(o->fname);
+       ast_free(o->fname);
 }
 
 
-static void set_fn(char *fn, int fn_size, const char *file, const char *configfile, struct ao2_container *fileset, struct inclfile **fi)
+static struct inclfile *set_fn(char *fn, int fn_size, const char *file, const char *configfile, struct ao2_container *fileset)
 {
        struct inclfile lookup;
-       
-       if (!file || file[0] == 0) {
+       struct inclfile *fi;
+
+       if (ast_strlen_zero(file)) {
                if (configfile[0] == '/')
                        ast_copy_string(fn, configfile, fn_size);
                else
                        snprintf(fn, fn_size, "%s/%s", ast_config_AST_CONFIG_DIR, configfile);
-       } else if (file[0] == '/') 
+       } else if (file[0] == '/')
                ast_copy_string(fn, file, fn_size);
        else
                snprintf(fn, fn_size, "%s/%s", ast_config_AST_CONFIG_DIR, file);
        lookup.fname = fn;
-       *fi = ao2_find(fileset, &lookup, OBJ_POINTER);
-       if (!(*fi)) {
-               /* set up a file scratch pad */
-               struct inclfile *fx = ao2_alloc(sizeof(struct inclfile), inclfile_destroy);
-               fx->fname = ast_strdup(fn);
-               fx->lineno = 1;
-               *fi = fx;
-               ao2_link(fileset, fx);
+       fi = ao2_find(fileset, &lookup, OBJ_POINTER);
+       if (fi) {
+               /* Found existing include file scratch pad. */
+               return fi;
        }
+
+       /* set up a file scratch pad */
+       fi = ao2_alloc(sizeof(struct inclfile), inclfile_destroy);
+       if (!fi) {
+               /* Scratch pad creation failed. */
+               return NULL;
+       }
+       fi->fname = ast_strdup(fn);
+       if (!fi->fname) {
+               /* Scratch pad creation failed. */
+               ao2_ref(fi, -1);
+               return NULL;
+       }
+       fi->lineno = 1;
+
+       ao2_link(fileset, fi);
+
+       return fi;
 }
 
 static int count_linefeeds(char *str)
@@ -1583,9 +1739,16 @@ static int count_linefeeds_in_comments(struct ast_comment *x)
 
 static void insert_leading_blank_lines(FILE *fp, struct inclfile *fi, struct ast_comment *precomments, int lineno)
 {
-       int precomment_lines = count_linefeeds_in_comments(precomments);
+       int precomment_lines;
        int i;
 
+       if (!fi) {
+               /* No file scratch pad object so insert no blank lines. */
+               return;
+       }
+
+       precomment_lines = count_linefeeds_in_comments(precomments);
+
        /* I don't have to worry about those ;! comments, they are
           stored in the precomments, but not printed back out.
           I did have to make sure that comments following
@@ -1618,65 +1781,76 @@ int config_text_file_save(const char *configfile, const struct ast_config *cfg,
 int ast_config_text_file_save(const char *configfile, const struct ast_config *cfg, const char *generator)
 {
        FILE *f;
-       char fn[256];
+       char fn[PATH_MAX];
        struct ast_variable *var;
        struct ast_category *cat;
        struct ast_comment *cmt;
        struct ast_config_include *incl;
        int blanklines = 0;
-       struct ao2_container *fileset = ao2_container_alloc(180000, hash_string, hashtab_compare_strings);
-       struct inclfile *fi = 0;
+       struct ao2_container *fileset;
+       struct inclfile *fi;
 
-       /* reset all the output flags, in case this isn't our first time saving this data */
+       fileset = ao2_container_alloc(1023, hash_string, hashtab_compare_strings);
+       if (!fileset) {
+               /* Container creation failed. */
+               return -1;
+       }
 
-       for (incl=cfg->includes; incl; incl = incl->next)
+       /* reset all the output flags, in case this isn't our first time saving this data */
+       for (incl = cfg->includes; incl; incl = incl->next) {
                incl->output = 0;
+       }
 
        /* go thru all the inclusions and make sure all the files involved (configfile plus all its inclusions)
           are all truncated to zero bytes and have that nice header*/
-
-       for (incl=cfg->includes; incl; incl = incl->next)
-       {
+       for (incl = cfg->includes; incl; incl = incl->next) {
                if (!incl->exec) { /* leave the execs alone -- we'll write out the #exec directives, but won't zero out the include files or exec files*/
-                       FILE *f1;
-
-                       set_fn(fn, sizeof(fn), incl->included_file, configfile, fileset, &fi); /* normally, fn is just set to incl->included_file, prepended with config dir if relative */
-                       f1 = fopen(fn,"w");
-                       if (f1) {
-                               gen_header(f1, configfile, fn, generator);
-                               fclose(f1); /* this should zero out the file */
+                       /* normally, fn is just set to incl->included_file, prepended with config dir if relative */
+                       fi = set_fn(fn, sizeof(fn), incl->included_file, configfile, fileset);
+                       f = fopen(fn, "w");
+                       if (f) {
+                               gen_header(f, configfile, fn, generator);
+                               fclose(f); /* this should zero out the file */
                        } else {
                                ast_debug(1, "Unable to open for writing: %s\n", fn);
                                ast_verb(2, "Unable to write %s (%s)", fn, strerror(errno));
                        }
-                       ao2_ref(fi,-1); /* we are giving up this reference to the object ptd to by fi */
-                       fi = 0;
+                       if (fi) {
+                               ao2_ref(fi, -1);
+                       }
                }
        }
 
-       set_fn(fn, sizeof(fn), 0, configfile, fileset, &fi); /* just set fn to absolute ver of configfile */
-#ifdef __CYGWIN__      
-       if ((f = fopen(fn, "w+"))) {
+       /* just set fn to absolute ver of configfile */
+       fi = set_fn(fn, sizeof(fn), 0, configfile, fileset);
+       if (
+#ifdef __CYGWIN__
+               (f = fopen(fn, "w+"))
 #else
-       if ((f = fopen(fn, "w"))) {
-#endif     
+               (f = fopen(fn, "w"))
+#endif
+               ) {
                ast_verb(2, "Saving '%s': ", fn);
                gen_header(f, configfile, fn, generator);
                cat = cfg->root;
                fclose(f);
-               ao2_ref(fi,-1); /* we are giving up this reference to the object ptd to by fi */
-               
+               if (fi) {
+                       ao2_ref(fi, -1);
+               }
+
                /* from here out, we open each involved file and concat the stuff we need to add to the end and immediately close... */
-               /* since each var, cat, and associated comments can come from any file, we have to be 
+               /* since each var, cat, and associated comments can come from any file, we have to be
                   mobile, and open each file, print, and close it on an entry-by-entry basis */
 
                while (cat) {
-                       set_fn(fn, sizeof(fn), cat->file, configfile, fileset, &fi);
+                       fi = set_fn(fn, sizeof(fn), cat->file, configfile, fileset);
                        f = fopen(fn, "a");
-                       if (!f)
-                       {
+                       if (!f) {
                                ast_debug(1, "Unable to open for writing: %s\n", fn);
                                ast_verb(2, "Unable to write %s (%s)", fn, strerror(errno));
+                               if (fi) {
+                                       ao2_ref(fi, -1);
+                               }
                                ao2_ref(fileset, -1);
                                return -1;
                        }
@@ -1737,9 +1911,10 @@ int ast_config_text_file_save(const char *configfile, const struct ast_config *c
                                        fprintf(f,"%s", cmt->cmt);
                        }
                        fclose(f);
-                       ao2_ref(fi,-1); /* we are giving up this reference to the object ptd to by fi */
-                       fi = 0;
-                       
+                       if (fi) {
+                               ao2_ref(fi, -1);
+                       }
+
                        var = cat->root;
                        while (var) {
                                struct ast_category_template_instance *x;
@@ -1759,18 +1934,18 @@ int ast_config_text_file_save(const char *configfile, const struct ast_config *c
                                        var = var->next;
                                        continue;
                                }
-                               set_fn(fn, sizeof(fn), var->file, configfile, fileset, &fi);
+                               fi = set_fn(fn, sizeof(fn), var->file, configfile, fileset);
                                f = fopen(fn, "a");
-                               if (!f)
-                               {
+                               if (!f) {
                                        ast_debug(1, "Unable to open for writing: %s\n", fn);
                                        ast_verb(2, "Unable to write %s (%s)", fn, strerror(errno));
-                                       ao2_ref(fi,-1); /* we are giving up this reference to the object ptd to by fi */
-                                       fi = 0;
+                                       if (fi) {
+                                               ao2_ref(fi, -1);
+                                       }
                                        ao2_ref(fileset, -1);
                                        return -1;
                                }
-                               
+
                                /* dump any includes that happen before this category header */
                                for (incl=cfg->includes; incl; incl = incl->next) {
                                        if (strcmp(incl->include_location_file, var->file) == 0){
@@ -1783,15 +1958,15 @@ int ast_config_text_file_save(const char *configfile, const struct ast_config *c
                                                }
                                        }
                                }
-                               
+
                                insert_leading_blank_lines(f, fi, var->precomments, var->lineno);
                                for (cmt = var->precomments; cmt; cmt=cmt->next) {
                                        if (cmt->cmt[0] != ';' || cmt->cmt[1] != '!')
                                                fprintf(f,"%s", cmt->cmt);
                                }
-                               if (var->sameline) 
+                               if (var->sameline)
                                        fprintf(f, "%s %s %s  %s", var->name, (var->object ? "=>" : "="), var->value, var->sameline->cmt);
-                               else    
+                               else
                                        fprintf(f, "%s %s %s\n", var->name, (var->object ? "=>" : "="), var->value);
                                for (cmt = var->trailing; cmt; cmt=cmt->next) {
                                        if (cmt->cmt[0] != ';' || cmt->cmt[1] != '!')
@@ -1802,11 +1977,12 @@ int ast_config_text_file_save(const char *configfile, const struct ast_config *c
                                        while (blanklines--)
                                                fprintf(f, "\n");
                                }
-                               
+
                                fclose(f);
-                               ao2_ref(fi,-1); /* we are giving up this reference to the object ptd to by fi */
-                               fi = 0;
-                               
+                               if (fi) {
+                                       ao2_ref(fi, -1);
+                               }
+
                                var = var->next;
                        }
                        cat = cat->next;
@@ -1816,29 +1992,30 @@ int ast_config_text_file_save(const char *configfile, const struct ast_config *c
        } else {
                ast_debug(1, "Unable to open for writing: %s\n", fn);
                ast_verb(2, "Unable to write (%s)", strerror(errno));
-               ao2_ref(fi,-1); /* we are giving up this reference to the object ptd to by fi */
+               if (fi) {
+                       ao2_ref(fi, -1);
+               }
                ao2_ref(fileset, -1);
                return -1;
        }
 
        /* Now, for files with trailing #include/#exec statements,
           we have to make sure every entry is output */
-
        for (incl=cfg->includes; incl; incl = incl->next) {
                if (!incl->output) {
                        /* open the respective file */
-                       set_fn(fn, sizeof(fn), incl->include_location_file, configfile, fileset, &fi);
+                       fi = set_fn(fn, sizeof(fn), incl->include_location_file, configfile, fileset);
                        f = fopen(fn, "a");
-                       if (!f)
-                       {
+                       if (!f) {
                                ast_debug(1, "Unable to open for writing: %s\n", fn);
                                ast_verb(2, "Unable to write %s (%s)", fn, strerror(errno));
-                               ao2_ref(fi,-1); /* we are giving up this reference to the object ptd to by fi */
-                               fi = 0;
+                               if (fi) {
+                                       ao2_ref(fi, -1);
+                               }
                                ao2_ref(fileset, -1);
                                return -1;
                        }
-                       
+
                        /* output the respective include */
                        if (incl->exec)
                                fprintf(f,"#exec \"%s\"\n", incl->exec_file);
@@ -1846,12 +2023,13 @@ int ast_config_text_file_save(const char *configfile, const struct ast_config *c
                                fprintf(f,"#include \"%s\"\n", incl->included_file);
                        fclose(f);
                        incl->output = 1;
-                       ao2_ref(fi,-1); /* we are giving up this reference to the object ptd to by fi */
-                       fi = 0;
+                       if (fi) {
+                               ao2_ref(fi, -1);
+                       }
                }
        }
        ao2_ref(fileset, -1); /* this should destroy the hash container */
-                               
+
        return 0;
 }
 
@@ -1873,6 +2051,7 @@ static void clear_config_maps(void)
 static int append_mapping(const char *name, const char *driver, const char *database, const char *table, int priority)
 {
        struct ast_config_map *map;
+       char *dst;
        int length;
 
        length = sizeof(*map);
@@ -1885,22 +2064,22 @@ static int append_mapping(const char *name, const char *driver, const char *data
        if (!(map = ast_calloc(1, length)))
                return -1;
 
-       map->name = map->stuff;
-       strcpy(map->name, name);
-       map->driver = map->name + strlen(map->name) + 1;
-       strcpy(map->driver, driver);
-       map->database = map->driver + strlen(map->driver) + 1;
-       strcpy(map->database, database);
+       dst = map->stuff;       /* writable space starts here */
+       map->name = strcpy(dst, name);
+       dst += strlen(dst) + 1;
+       map->driver = strcpy(dst, driver);
+       dst += strlen(dst) + 1;
+       map->database = strcpy(dst, database);
        if (table) {
-               map->table = map->database + strlen(map->database) + 1;
-               strcpy(map->table, table);
+               dst += strlen(dst) + 1;
+               map->table = strcpy(dst, table);
        }
        map->priority = priority;
        map->next = config_maps;
+       config_maps = map;
 
        ast_verb(2, "Binding %s to %s/%s/%s\n", map->name, map->driver, map->database, map->table ? map->table : map->name);
 
-       config_maps = map;
        return 0;
 }