Merged revisions 334297 via svnmerge from

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

1 files changed, 338 insertions, 159 deletions

diff --git a/main/config.c b/main/config.c
index 43a109ad2..e432de33d 100644
--- a/main/config.c
+++ b/main/config.c
@@ -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;
 }