we can now build with -Wformat=2, which found a couple of real bugs

because SPRINTF() use non-literal format strings (which cannot be checked), move it into its own module so the rest of func_strings can benefit from format string checking



git-svn-id: http://svn.digium.com/svn/asterisk/trunk@159774 f38db490-d61c-443f-a65b-d21fe96a405b

9 files changed, 261 insertions, 188 deletions

diff --git a/Makefile b/Makefile
index 45b01b769..39b17c5f3 100644
--- a/Makefile
+++ b/Makefile
@@ -237,7 +237,7 @@ endif
 ASTCFLAGS+=-Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations $(DEBUG)
 
 ifeq ($(AST_DEVMODE),yes)
-  ASTCFLAGS+=-Werror -Wunused -Wundef $(AST_DECLARATION_AFTER_STATEMENT) -Wmissing-format-attribute -Wformat-security #-Wformat=2
+  ASTCFLAGS+=-Werror -Wunused -Wundef $(AST_DECLARATION_AFTER_STATEMENT) -Wmissing-format-attribute -Wformat=2
 endif
 
 ifneq ($(findstring BSD,$(OSARCH)),)
diff --git a/UPGRADE.txt b/UPGRADE.txt
index 199da5ed5..ccfe1c510 100644
--- a/UPGRADE.txt
+++ b/UPGRADE.txt
@@ -85,7 +85,7 @@ From 1.6.1 to 1.6.2:
   has been replaced with 'UNSUPPORTED').  This change makes the
   SendImage application more consistent with other applications.
 
-* skinny.conf now has seperate sections for lines and devices.
+* skinny.conf now has separate sections for lines and devices.
   Please have a look at configs/skinny.conf.sample and update
   your skinny.conf.
 
@@ -95,3 +95,8 @@ From 1.6.1 to 1.6.2:
   case-insensitive comparisons now when originally hashing based on
   queue names, meaning that now the two queues mentioned as examples
   earlier will be seen as having the same name.
+
+* The SPRINTF() dialplan function has been moved into its own module,
+  func_sprintf, and is no longer included in func_strings. If you use this
+  function and do not use 'autoload=yes' in modules.conf, you will need
+  to explicitly load func_sprintf for it to be available.
diff --git a/channels/misdn/ie.c b/channels/misdn/ie.c
index fbd06d163..dc9802845 100644
--- a/channels/misdn/ie.c
+++ b/channels/misdn/ie.c
@@ -1347,7 +1347,7 @@ static void enc_ie_useruser(unsigned char **ntmode, msg_t *msg, int protocol, ch
 	i = 0;
 	while(i < user_len)
 	{
-		if (MISDN_IE_DEBG) printf(debug+(i*3), " %02x", user[i]);
+		if (MISDN_IE_DEBG) sprintf(debug+(i*3), " %02x", user[i]);
 		i++;
 	}
 		
@@ -1393,7 +1393,7 @@ static void dec_ie_useruser(unsigned char *p, Q931_info_t *qi, int *protocol, ch
 	i = 0;
 	while(i < *user_len)
 	{
-		if (MISDN_IE_DEBG) printf(debug+(i*3), " %02x", user[i]);
+		if (MISDN_IE_DEBG) sprintf(debug+(i*3), " %02x", user[i]);
 		i++;
 	}
 	debug[i*3] = '\0';
diff --git a/channels/misdn_config.c b/channels/misdn_config.c
index 12a742cf3..29723e17c 100644
--- a/channels/misdn_config.c
+++ b/channels/misdn_config.c
@@ -882,12 +882,14 @@ static int _parse (union misdn_cfg_pt *dest, const char *value, enum misdn_cfg_t
 		break;
 	case MISDN_CTYPE_INT:
 	{
-		char *pat;
-		if (strchr(value,'x')) 
-			pat="%x";
-		else
-			pat="%d";
-		if (sscanf(value, pat, &tmp)) {
+		int res;
+
+		if (strchr(value,'x')) {
+			res = sscanf(value, "%x", &tmp);
+		} else {
+			res = sscanf(value, "%d", &tmp);
+		}
+		if (res) {
 			dest->num = ast_malloc(sizeof(int));
 			memcpy(dest->num, &tmp, sizeof(int));
 		} else
diff --git a/funcs/Makefile b/funcs/Makefile
index 9b13b17a0..bc4745d08 100644
--- a/funcs/Makefile
+++ b/funcs/Makefile
@@ -18,3 +18,10 @@ MENUSELECT_DESCRIPTION=Dialplan Functions
 all: _all
 
 include $(ASTTOPDIR)/Makefile.moddir_rules
+
+# the SPRINTF() function in func_sprintf accepts format specifiers
+# and thus passes them to snprintf() as non-literal strings; the compiler
+# can't check the string and arguments to ensure they match, so this
+# warning must be disabled; for safety reasons, SPRINTF() is kept in
+# a separate module so that as little code as possible is left unchecked
+func_sprintf.o: ASTCFLAGS+=-Wno-format-nonliteral
diff --git a/funcs/func_sprintf.c b/funcs/func_sprintf.c
new file mode 100644
index 000000000..af292df1d
--- /dev/null
+++ b/funcs/func_sprintf.c
@@ -0,0 +1,230 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2005-2006, Digium, Inc.
+ * Portions Copyright (C) 2005, Tilghman Lesher.  All rights reserved.
+ * Portions Copyright (C) 2005, Anthony Minessale II
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+/*! \file
+ *
+ * \brief String manipulation dialplan functions
+ *
+ * \author Tilghman Lesher
+ * \author Anothony Minessale II 
+ * \ingroup functions
+ */
+
+#include "asterisk.h"
+
+ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
+
+#include <ctype.h>
+
+#include "asterisk/module.h"
+#include "asterisk/channel.h"
+#include "asterisk/pbx.h"
+#include "asterisk/utils.h"
+#include "asterisk/app.h"
+
+AST_THREADSTORAGE(result_buf);
+
+/*** DOCUMENTATION
+	<function name="SPRINTF" language="en_US">
+		<synopsis>
+			Format a variable according to a format string.
+		</synopsis>
+		<syntax>
+			<parameter name="format" required="true" />
+			<parameter name="arg1" required="true" />
+			<parameter name="arg2" multiple="true" />
+			<parameter name="argN" />
+		</syntax>
+		<description>
+			<para>Parses the format string specified and returns a string matching 
+			that format. Supports most options found in <emphasis>sprintf(3)</emphasis>.
+			Returns a shortened string if a format specifier is not recognized.</para>
+		</description>
+		<see-also>
+			<ref type="manpage">sprintf(3)</ref>
+		</see-also>
+	</function>
+ ***/
+static int acf_sprintf(struct ast_channel *chan, const char *cmd, char *data, char *buf, size_t len)
+{
+#define SPRINTF_FLAG	0
+#define SPRINTF_WIDTH	1
+#define SPRINTF_PRECISION	2
+#define SPRINTF_LENGTH	3
+#define SPRINTF_CONVERSION	4
+	int i, state = -1, argcount = 0;
+	char *formatstart = NULL, *bufptr = buf;
+	char formatbuf[256] = "";
+	int tmpi;
+	double tmpd;
+	AST_DECLARE_APP_ARGS(arg,
+				AST_APP_ARG(format);
+				AST_APP_ARG(var)[100];
+	);
+
+	AST_STANDARD_APP_ARGS(arg, data);
+
+	/* Scan the format, converting each argument into the requisite format type. */
+	for (i = 0; arg.format[i]; i++) {
+		switch (state) {
+		case SPRINTF_FLAG:
+			if (strchr("#0- +'I", arg.format[i]))
+				break;
+			state = SPRINTF_WIDTH;
+		case SPRINTF_WIDTH:
+			if (arg.format[i] >= '0' && arg.format[i] <= '9')
+				break;
+
+			/* Next character must be a period to go into a precision */
+			if (arg.format[i] == '.') {
+				state = SPRINTF_PRECISION;
+			} else {
+				state = SPRINTF_LENGTH;
+				i--;
+			}
+			break;
+		case SPRINTF_PRECISION:
+			if (arg.format[i] >= '0' && arg.format[i] <= '9')
+				break;
+			state = SPRINTF_LENGTH;
+		case SPRINTF_LENGTH:
+			if (strchr("hl", arg.format[i])) {
+				if (arg.format[i + 1] == arg.format[i])
+					i++;
+				state = SPRINTF_CONVERSION;
+				break;
+			} else if (strchr("Lqjzt", arg.format[i])) {
+				state = SPRINTF_CONVERSION;
+				break;
+			}
+			state = SPRINTF_CONVERSION;
+		case SPRINTF_CONVERSION:
+			if (strchr("diouxXc", arg.format[i])) {
+				/* Integer */
+
+				/* Isolate this format alone */
+				ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
+				formatbuf[&arg.format[i] - formatstart + 1] = '\0';
+
+				/* Convert the argument into the required type */
+				if (arg.var[argcount]) {
+					if (sscanf(arg.var[argcount++], "%d", &tmpi) != 1) {
+						ast_log(LOG_ERROR, "Argument '%s' is not an integer number for format '%s'\n", arg.var[argcount - 1], formatbuf);
+						goto sprintf_fail;
+					}
+				} else {
+					ast_log(LOG_ERROR, "SPRINTF() has more format specifiers than arguments!\n");
+					goto sprintf_fail;
+				}
+
+				/* Format the argument */
+				snprintf(bufptr, buf + len - bufptr, formatbuf, tmpi);
+
+				/* Update the position of the next parameter to print */
+				bufptr = strchr(buf, '\0');
+			} else if (strchr("eEfFgGaA", arg.format[i])) {
+				/* Double */
+
+				/* Isolate this format alone */
+				ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
+				formatbuf[&arg.format[i] - formatstart + 1] = '\0';
+
+				/* Convert the argument into the required type */
+				if (arg.var[argcount]) {
+					if (sscanf(arg.var[argcount++], "%lf", &tmpd) != 1) {
+						ast_log(LOG_ERROR, "Argument '%s' is not a floating point number for format '%s'\n", arg.var[argcount - 1], formatbuf);
+						goto sprintf_fail;
+					}
+				} else {
+					ast_log(LOG_ERROR, "SPRINTF() has more format specifiers than arguments!\n");
+					goto sprintf_fail;
+				}
+
+				/* Format the argument */
+				snprintf(bufptr, buf + len - bufptr, formatbuf, tmpd);
+
+				/* Update the position of the next parameter to print */
+				bufptr = strchr(buf, '\0');
+			} else if (arg.format[i] == 's') {
+				/* String */
+
+				/* Isolate this format alone */
+				ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
+				formatbuf[&arg.format[i] - formatstart + 1] = '\0';
+
+				/* Format the argument */
+				snprintf(bufptr, buf + len - bufptr, formatbuf, arg.var[argcount++]);
+
+				/* Update the position of the next parameter to print */
+				bufptr = strchr(buf, '\0');
+			} else if (arg.format[i] == '%') {
+				/* Literal data to copy */
+				*bufptr++ = arg.format[i];
+			} else {
+				/* Not supported */
+
+				/* Isolate this format alone */
+				ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
+				formatbuf[&arg.format[i] - formatstart + 1] = '\0';
+
+				ast_log(LOG_ERROR, "Format type not supported: '%s' with argument '%s'\n", formatbuf, arg.var[argcount++]);
+				goto sprintf_fail;
+			}
+			state = -1;
+			break;
+		default:
+			if (arg.format[i] == '%') {
+				state = SPRINTF_FLAG;
+				formatstart = &arg.format[i];
+				break;
+			} else {
+				/* Literal data to copy */
+				*bufptr++ = arg.format[i];
+			}
+		}
+	}
+	*bufptr = '\0';
+	return 0;
+sprintf_fail:
+	return -1;
+}
+
+static struct ast_custom_function sprintf_function = {
+	.name = "SPRINTF",
+	.read = acf_sprintf,
+};
+
+static int unload_module(void)
+{
+	int res = 0;
+
+	res |= ast_custom_function_unregister(&sprintf_function);
+
+	return res;
+}
+
+static int load_module(void)
+{
+	int res = 0;
+
+	res |= ast_custom_function_register(&sprintf_function);
+
+	return res;
+}
+
+AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "SPRINTF dialplan function");
diff --git a/funcs/func_strings.c b/funcs/func_strings.c
index 2e254ab87..350a5ea19 100644
--- a/funcs/func_strings.c
+++ b/funcs/func_strings.c
@@ -260,25 +260,6 @@ AST_THREADSTORAGE(result_buf);
 			<para>Example: ${LEN(example)} returns 7</para>
 		</description>
 	</function>
-	<function name="SPRINTF" language="en_US">
-		<synopsis>
-			Format a variable according to a format string.
-		</synopsis>
-		<syntax>
-			<parameter name="format" required="true" />
-			<parameter name="arg1" required="true" />
-			<parameter name="arg2" multiple="true" />
-			<parameter name="argN" />
-		</syntax>
-		<description>
-			<para>Parses the format string specified and returns a string matching 
-			that format. Supports most options found in <emphasis>sprintf(3)</emphasis>.
-			Returns a shortened string if a format specifier is not recognized.</para>
-		</description>
-		<see-also>
-			<ref type="manpage">sprintf(3)</ref>
-		</see-also>
-	</function>
 	<function name="QUOTE" language="en_US">
 		<synopsis>
 			Quotes a given string, escaping embedded quotes as necessary
@@ -744,155 +725,6 @@ static struct ast_custom_function array_function = {
 	.write = array,
 };
 
-static int acf_sprintf(struct ast_channel *chan, const char *cmd, char *data, char *buf, size_t len)
-{
-#define SPRINTF_FLAG	0
-#define SPRINTF_WIDTH	1
-#define SPRINTF_PRECISION	2
-#define SPRINTF_LENGTH	3
-#define SPRINTF_CONVERSION	4
-	int i, state = -1, argcount = 0;
-	char *formatstart = NULL, *bufptr = buf;
-	char formatbuf[256] = "";
-	int tmpi;
-	double tmpd;
-	AST_DECLARE_APP_ARGS(arg,
-				AST_APP_ARG(format);
-				AST_APP_ARG(var)[100];
-	);
-
-	AST_STANDARD_APP_ARGS(arg, data);
-
-	/* Scan the format, converting each argument into the requisite format type. */
-	for (i = 0; arg.format[i]; i++) {
-		switch (state) {
-		case SPRINTF_FLAG:
-			if (strchr("#0- +'I", arg.format[i]))
-				break;
-			state = SPRINTF_WIDTH;
-		case SPRINTF_WIDTH:
-			if (arg.format[i] >= '0' && arg.format[i] <= '9')
-				break;
-
-			/* Next character must be a period to go into a precision */
-			if (arg.format[i] == '.') {
-				state = SPRINTF_PRECISION;
-			} else {
-				state = SPRINTF_LENGTH;
-				i--;
-			}
-			break;
-		case SPRINTF_PRECISION:
-			if (arg.format[i] >= '0' && arg.format[i] <= '9')
-				break;
-			state = SPRINTF_LENGTH;
-		case SPRINTF_LENGTH:
-			if (strchr("hl", arg.format[i])) {
-				if (arg.format[i + 1] == arg.format[i])
-					i++;
-				state = SPRINTF_CONVERSION;
-				break;
-			} else if (strchr("Lqjzt", arg.format[i])) {
-				state = SPRINTF_CONVERSION;
-				break;
-			}
-			state = SPRINTF_CONVERSION;
-		case SPRINTF_CONVERSION:
-			if (strchr("diouxXc", arg.format[i])) {
-				/* Integer */
-
-				/* Isolate this format alone */
-				ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
-				formatbuf[&arg.format[i] - formatstart + 1] = '\0';
-
-				/* Convert the argument into the required type */
-				if (arg.var[argcount]) {
-					if (sscanf(arg.var[argcount++], "%d", &tmpi) != 1) {
-						ast_log(LOG_ERROR, "Argument '%s' is not an integer number for format '%s'\n", arg.var[argcount - 1], formatbuf);
-						goto sprintf_fail;
-					}
-				} else {
-					ast_log(LOG_ERROR, "SPRINTF() has more format specifiers than arguments!\n");
-					goto sprintf_fail;
-				}
-
-				/* Format the argument */
-				snprintf(bufptr, buf + len - bufptr, formatbuf, tmpi);
-
-				/* Update the position of the next parameter to print */
-				bufptr = strchr(buf, '\0');
-			} else if (strchr("eEfFgGaA", arg.format[i])) {
-				/* Double */
-
-				/* Isolate this format alone */
-				ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
-				formatbuf[&arg.format[i] - formatstart + 1] = '\0';
-
-				/* Convert the argument into the required type */
-				if (arg.var[argcount]) {
-					if (sscanf(arg.var[argcount++], "%lf", &tmpd) != 1) {
-						ast_log(LOG_ERROR, "Argument '%s' is not a floating point number for format '%s'\n", arg.var[argcount - 1], formatbuf);
-						goto sprintf_fail;
-					}
-				} else {
-					ast_log(LOG_ERROR, "SPRINTF() has more format specifiers than arguments!\n");
-					goto sprintf_fail;
-				}
-
-				/* Format the argument */
-				snprintf(bufptr, buf + len - bufptr, formatbuf, tmpd);
-
-				/* Update the position of the next parameter to print */
-				bufptr = strchr(buf, '\0');
-			} else if (arg.format[i] == 's') {
-				/* String */
-
-				/* Isolate this format alone */
-				ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
-				formatbuf[&arg.format[i] - formatstart + 1] = '\0';
-
-				/* Format the argument */
-				snprintf(bufptr, buf + len - bufptr, formatbuf, arg.var[argcount++]);
-
-				/* Update the position of the next parameter to print */
-				bufptr = strchr(buf, '\0');
-			} else if (arg.format[i] == '%') {
-				/* Literal data to copy */
-				*bufptr++ = arg.format[i];
-			} else {
-				/* Not supported */
-
-				/* Isolate this format alone */
-				ast_copy_string(formatbuf, formatstart, sizeof(formatbuf));
-				formatbuf[&arg.format[i] - formatstart + 1] = '\0';
-
-				ast_log(LOG_ERROR, "Format type not supported: '%s' with argument '%s'\n", formatbuf, arg.var[argcount++]);
-				goto sprintf_fail;
-			}
-			state = -1;
-			break;
-		default:
-			if (arg.format[i] == '%') {
-				state = SPRINTF_FLAG;
-				formatstart = &arg.format[i];
-				break;
-			} else {
-				/* Literal data to copy */
-				*bufptr++ = arg.format[i];
-			}
-		}
-	}
-	*bufptr = '\0';
-	return 0;
-sprintf_fail:
-	return -1;
-}
-
-static struct ast_custom_function sprintf_function = {
-	.name = "SPRINTF",
-	.read = acf_sprintf,
-};
-
 static int quote(struct ast_channel *chan, const char *cmd, char *data, char *buf, size_t len)
 {
 	char *bufptr = buf, *dataptr = data;
@@ -1120,7 +952,6 @@ static int unload_module(void)
 	res |= ast_custom_function_unregister(&strptime_function);
 	res |= ast_custom_function_unregister(&eval_function);
 	res |= ast_custom_function_unregister(&keypadhash_function);
-	res |= ast_custom_function_unregister(&sprintf_function);
 	res |= ast_custom_function_unregister(&hashkeys_function);
 	res |= ast_custom_function_unregister(&hash_function);
 	res |= ast_unregister_application(app_clearhash);
@@ -1145,7 +976,6 @@ static int load_module(void)
 	res |= ast_custom_function_register(&strptime_function);
 	res |= ast_custom_function_register(&eval_function);
 	res |= ast_custom_function_register(&keypadhash_function);
-	res |= ast_custom_function_register(&sprintf_function);
 	res |= ast_custom_function_register(&hashkeys_function);
 	res |= ast_custom_function_register(&hash_function);
 	res |= ast_register_application_xml(app_clearhash, exec_clearhash);
diff --git a/main/Makefile b/main/Makefile
index 51acf20ee..56b964d16 100644
--- a/main/Makefile
+++ b/main/Makefile
@@ -142,7 +142,7 @@ ifneq ($(findstring ENABLE_UPLOADS,$(MENUSELECT_CFLAGS)),)
 http.o: ASTCFLAGS+=$(GMIME_INCLUDE)
 endif
 
-stdtime/localtime.o: ASTCFLAGS+=$(AST_NO_STRICT_OVERFLOW)
+stdtime/localtime.o: ASTCFLAGS+=$(AST_NO_STRICT_OVERFLOW) -Wno-format-nonliteral
 
 AST_EMBED_LDSCRIPTS:=$(sort $(EMBED_LDSCRIPTS))
 AST_EMBED_LDFLAGS:=$(foreach dep,$(EMBED_LDFLAGS),$(value $(dep)))
diff --git a/res/res_config_sqlite.c b/res/res_config_sqlite.c
index ff3c416cc..5f72abc9a 100644
--- a/res/res_config_sqlite.c
+++ b/res/res_config_sqlite.c
@@ -555,8 +555,7 @@ static char *sql_create_cdr_table =
 /*!
  * SQL query format to describe the table structure
  */
-static char *sql_table_structure =
-"SELECT sql FROM sqlite_master WHERE type='table' AND tbl_name='%s'";
+#define sql_table_structure "SELECT sql FROM sqlite_master WHERE type='table' AND tbl_name='%s'"
 
 /*!
  * SQL query format to fetch the static configuration of a file.
@@ -564,11 +563,11 @@ static char *sql_table_structure =
  *
  * \see add_cfg_entry()
  */
-static const char *sql_get_config_table =
-"SELECT *"
-"	FROM '%q'"
-"	WHERE filename = '%q' AND commented = 0"
-"	ORDER BY cat_metric ASC, var_metric ASC;";
+#define sql_get_config_table \
+	"SELECT *" \
+	"	FROM '%q'" \
+	"	WHERE filename = '%q' AND commented = 0" \
+	"	ORDER BY cat_metric ASC, var_metric ASC;"
 
 static void free_table(struct sqlite_cache_tables *tblptr)
 {