we can now build with -Wformat=2, which found a couple of real bugs
authorKevin P. Fleming <kpfleming@digium.com>
Sat, 29 Nov 2008 15:29:33 +0000 (15:29 +0000)
committerKevin P. Fleming <kpfleming@digium.com>
Sat, 29 Nov 2008 15:29:33 +0000 (15:29 +0000)
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: https://origsvn.digium.com/svn/asterisk/trunk@159774 65c4cc65-6c06-0410-ace0-fbb531ad65f3

Makefile
UPGRADE.txt
channels/misdn/ie.c
channels/misdn_config.c
funcs/Makefile
funcs/func_sprintf.c [new file with mode: 0644]
funcs/func_strings.c
main/Makefile
res/res_config_sqlite.c

index 45b01b7..39b17c5 100644 (file)
--- 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)),)
index 199da5e..ccfe1c5 100644 (file)
@@ -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.
index fbd06d1..dc98028 100644 (file)
@@ -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';
index 12a742c..29723e1 100644 (file)
@@ -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
index 9b13b17..bc4745d 100644 (file)
@@ -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 (file)
index 0000000..af292df
--- /dev/null
@@ -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");
index 2e254ab..350a5ea 100644 (file)
@@ -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);
index 51acf20..56b964d 100644 (file)
@@ -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)))
index ff3c416..5f72abc 100644 (file)
@@ -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)
 {