From 52ce20589a8bac4eccaea043b1fe283daaf4f9e3 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Sat, 10 Sep 2011 23:12:46 +0300 Subject: [PATCH] Add missing format attributes Add __attribute__ decorations for printf format checking to the places that were missing them. Fix the resulting warnings. Add -Wmissing-format-attribute to the standard set of warnings for GCC, so these don't happen again. The warning fixes here are relatively harmless. The one serious problem discovered by this was already committed earlier in cf15fb5cabfbc71e07be23cfbc813daee6c5014f. --- configure | 60 ++++++++++++++++++++++++ configure.in | 1 + contrib/pg_upgrade/info.c | 2 +- contrib/pg_upgrade/pg_upgrade.h | 16 ++++--- contrib/pg_upgrade/relfilenode.c | 4 +- contrib/pgcrypto/px.h | 3 +- src/bin/pg_dump/pg_backup_archiver.c | 4 +- src/bin/pg_dump/pg_backup_tar.c | 2 +- src/include/lib/stringinfo.h | 3 +- src/interfaces/ecpg/ecpglib/descriptor.c | 4 +- src/interfaces/ecpg/ecpglib/error.c | 2 +- src/interfaces/ecpg/ecpglib/execute.c | 2 +- src/interfaces/ecpg/ecpglib/extern.h | 2 +- 13 files changed, 87 insertions(+), 18 deletions(-) diff --git a/configure b/configure index 31bc8fb374b..58fea907ea6 100755 --- a/configure +++ b/configure @@ -4123,6 +4123,66 @@ if test x"$pgac_cv_prog_cc_cflags__Wendif_labels" = x"yes"; then CFLAGS="$CFLAGS -Wendif-labels" fi + { $as_echo "$as_me:$LINENO: checking whether $CC supports -Wmissing-format-attribute" >&5 +$as_echo_n "checking whether $CC supports -Wmissing-format-attribute... " >&6; } +if test "${pgac_cv_prog_cc_cflags__Wmissing_format_attribute+set}" = set; then + $as_echo_n "(cached) " >&6 +else + pgac_save_CFLAGS=$CFLAGS +CFLAGS="$pgac_save_CFLAGS -Wmissing-format-attribute" +ac_save_c_werror_flag=$ac_c_werror_flag +ac_c_werror_flag=yes +cat >conftest.$ac_ext <<_ACEOF +/* confdefs.h. */ +_ACEOF +cat confdefs.h >>conftest.$ac_ext +cat >>conftest.$ac_ext <<_ACEOF +/* end confdefs.h. */ + +int +main () +{ + + ; + return 0; +} +_ACEOF +rm -f conftest.$ac_objext +if { (ac_try="$ac_compile" +case "(($ac_try" in + *\"* | *\`* | *\\*) ac_try_echo=\$ac_try;; + *) ac_try_echo=$ac_try;; +esac +eval ac_try_echo="\"\$as_me:$LINENO: $ac_try_echo\"" +$as_echo "$ac_try_echo") >&5 + (eval "$ac_compile") 2>conftest.er1 + ac_status=$? + grep -v '^ *+' conftest.er1 >conftest.err + rm -f conftest.er1 + cat conftest.err >&5 + $as_echo "$as_me:$LINENO: \$? = $ac_status" >&5 + (exit $ac_status); } && { + test -z "$ac_c_werror_flag" || + test ! -s conftest.err + } && test -s conftest.$ac_objext; then + pgac_cv_prog_cc_cflags__Wmissing_format_attribute=yes +else + $as_echo "$as_me: failed program was:" >&5 +sed 's/^/| /' conftest.$ac_ext >&5 + + pgac_cv_prog_cc_cflags__Wmissing_format_attribute=no +fi + +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +ac_c_werror_flag=$ac_save_c_werror_flag +CFLAGS="$pgac_save_CFLAGS" +fi +{ $as_echo "$as_me:$LINENO: result: $pgac_cv_prog_cc_cflags__Wmissing_format_attribute" >&5 +$as_echo "$pgac_cv_prog_cc_cflags__Wmissing_format_attribute" >&6; } +if test x"$pgac_cv_prog_cc_cflags__Wmissing_format_attribute" = x"yes"; then + CFLAGS="$CFLAGS -Wmissing-format-attribute" +fi + # This was included in -Wall/-Wformat in older GCC versions { $as_echo "$as_me:$LINENO: checking whether $CC supports -Wformat-security" >&5 $as_echo_n "checking whether $CC supports -Wformat-security... " >&6; } diff --git a/configure.in b/configure.in index 05259cb8694..5dc669f545a 100644 --- a/configure.in +++ b/configure.in @@ -430,6 +430,7 @@ if test "$GCC" = yes -a "$ICC" = no; then # These work in some but not all gcc versions PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement]) PGAC_PROG_CC_CFLAGS_OPT([-Wendif-labels]) + PGAC_PROG_CC_CFLAGS_OPT([-Wmissing-format-attribute]) # This was included in -Wall/-Wformat in older GCC versions PGAC_PROG_CC_CFLAGS_OPT([-Wformat-security]) # Disable strict-aliasing rules; needed for gcc 3.3+ diff --git a/contrib/pg_upgrade/info.c b/contrib/pg_upgrade/info.c index 5606486ef36..e41ab2b1071 100644 --- a/contrib/pg_upgrade/info.c +++ b/contrib/pg_upgrade/info.c @@ -286,7 +286,7 @@ get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo) (GET_MAJOR_VERSION(old_cluster.major_version) <= 804) ? "" : ", 'pg_largeobject_metadata', 'pg_largeobject_metadata_oid_index'"); - res = executeQueryOrDie(conn, query); + res = executeQueryOrDie(conn, "%s", query); ntups = PQntuples(res); diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h index 2abd91771f9..46aed74450d 100644 --- a/contrib/pg_upgrade/pg_upgrade.h +++ b/contrib/pg_upgrade/pg_upgrade.h @@ -292,8 +292,8 @@ void split_old_dump(void); /* exec.c */ -int exec_prog(bool throw_error, - const char *cmd,...); +int exec_prog(bool throw_error, const char *cmd, ...) + __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3))); void verify_directories(void); bool is_server_running(const char *datadir); void rename_old_pg_control(void); @@ -377,7 +377,8 @@ void init_tablespaces(void); /* server.c */ PGconn *connectToServer(ClusterInfo *cluster, const char *db_name); -PGresult *executeQueryOrDie(PGconn *conn, const char *fmt,...); +PGresult *executeQueryOrDie(PGconn *conn, const char *fmt, ...) + __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3))); void start_postmaster(ClusterInfo *cluster); void stop_postmaster(bool fast); @@ -390,9 +391,12 @@ void check_pghost_envvar(void); char *quote_identifier(const char *s); int get_user_info(char **user_name); void check_ok(void); -void report_status(eLogType type, const char *fmt,...); -void pg_log(eLogType type, char *fmt,...); -void prep_status(const char *fmt,...); +void report_status(eLogType type, const char *fmt, ...) + __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3))); +void pg_log(eLogType type, char *fmt, ...) + __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3))); +void prep_status(const char *fmt, ...) + __attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 2))); void check_ok(void); char *pg_strdup(const char *s); void *pg_malloc(int size); diff --git a/contrib/pg_upgrade/relfilenode.c b/contrib/pg_upgrade/relfilenode.c index 9354d92d36c..1aefd337f1f 100644 --- a/contrib/pg_upgrade/relfilenode.c +++ b/contrib/pg_upgrade/relfilenode.c @@ -71,7 +71,9 @@ transfer_all_new_dbs(DbInfoArr *old_db_arr, } } - prep_status(""); /* in case nothing printed */ + prep_status(" "); /* in case nothing printed; pass a space so gcc + * doesn't complain about empty format + * string */ check_ok(); return msg; diff --git a/contrib/pgcrypto/px.h b/contrib/pgcrypto/px.h index 9709f9bdb60..610b7fad789 100644 --- a/contrib/pgcrypto/px.h +++ b/contrib/pgcrypto/px.h @@ -204,7 +204,8 @@ const char *px_resolve_alias(const PX_Alias *aliases, const char *name); void px_set_debug_handler(void (*handler) (const char *)); #ifdef PX_DEBUG -void px_debug(const char *fmt,...); +void px_debug(const char *fmt, ...) + __attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 2))); #else #define px_debug(...) #endif diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 62206ecda4d..1611551638b 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -119,8 +119,8 @@ static int _discoverArchiveFormat(ArchiveHandle *AH); static int RestoringToDB(ArchiveHandle *AH); static void dump_lo_buf(ArchiveHandle *AH); -static void _write_msg(const char *modulename, const char *fmt, va_list ap); -static void _die_horribly(ArchiveHandle *AH, const char *modulename, const char *fmt, va_list ap); +static void _write_msg(const char *modulename, const char *fmt, va_list ap) __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 0))); +static void _die_horribly(ArchiveHandle *AH, const char *modulename, const char *fmt, va_list ap) __attribute__((format(PG_PRINTF_ATTRIBUTE, 3, 0))); static void dumpTimestamp(ArchiveHandle *AH, const char *msg, time_t tim); static void SetOutput(ArchiveHandle *AH, char *filename, int compression); diff --git a/src/bin/pg_dump/pg_backup_tar.c b/src/bin/pg_dump/pg_backup_tar.c index 041f9f9cbbb..45aab1e2364 100644 --- a/src/bin/pg_dump/pg_backup_tar.c +++ b/src/bin/pg_dump/pg_backup_tar.c @@ -112,7 +112,7 @@ static void tarClose(ArchiveHandle *AH, TAR_MEMBER *TH); #ifdef __NOT_USED__ static char *tarGets(char *buf, size_t len, TAR_MEMBER *th); #endif -static int tarPrintf(ArchiveHandle *AH, TAR_MEMBER *th, const char *fmt,...); +static int tarPrintf(ArchiveHandle *AH, TAR_MEMBER *th, const char *fmt, ...) __attribute__((format(PG_PRINTF_ATTRIBUTE, 3, 4))); static void _tarAddFile(ArchiveHandle *AH, TAR_MEMBER *th); static int _tarChecksum(char *th); diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h index 3657a685395..79b09d23b43 100644 --- a/src/include/lib/stringinfo.h +++ b/src/include/lib/stringinfo.h @@ -105,7 +105,8 @@ __attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 3))); * without modifying str. Typically the caller would enlarge str and retry * on false return --- see appendStringInfo for standard usage pattern. */ -extern bool appendStringInfoVA(StringInfo str, const char *fmt, va_list args); +extern bool appendStringInfoVA(StringInfo str, const char *fmt, va_list args) +__attribute__((format(PG_PRINTF_ATTRIBUTE, 2, 0))); /*------------------------ * appendStringInfoString diff --git a/src/interfaces/ecpg/ecpglib/descriptor.c b/src/interfaces/ecpg/ecpglib/descriptor.c index 5f56de03120..fb5aa9a185d 100644 --- a/src/interfaces/ecpg/ecpglib/descriptor.c +++ b/src/interfaces/ecpg/ecpglib/descriptor.c @@ -388,7 +388,7 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...) */ if (arrsize > 0 && ntuples > arrsize) { - ecpg_log("ECPGget_desc on line %d: incorrect number of matches; %d don't fit into array of %d\n", + ecpg_log("ECPGget_desc on line %d: incorrect number of matches; %d don't fit into array of %ld\n", lineno, ntuples, arrsize); ecpg_raise(lineno, ECPG_TOO_MANY_MATCHES, ECPG_SQLSTATE_CARDINALITY_VIOLATION, NULL); return false; @@ -457,7 +457,7 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...) */ if (data_var.ind_arrsize > 0 && ntuples > data_var.ind_arrsize) { - ecpg_log("ECPGget_desc on line %d: incorrect number of matches (indicator); %d don't fit into array of %d\n", + ecpg_log("ECPGget_desc on line %d: incorrect number of matches (indicator); %d don't fit into array of %ld\n", lineno, ntuples, data_var.ind_arrsize); ecpg_raise(lineno, ECPG_TOO_MANY_MATCHES, ECPG_SQLSTATE_CARDINALITY_VIOLATION, NULL); return false; diff --git a/src/interfaces/ecpg/ecpglib/error.c b/src/interfaces/ecpg/ecpglib/error.c index 58024f47b0c..54fb6f94508 100644 --- a/src/interfaces/ecpg/ecpglib/error.c +++ b/src/interfaces/ecpg/ecpglib/error.c @@ -335,7 +335,7 @@ ecpg_raise_backend(int line, PGresult *result, PGconn *conn, int compat) sqlca->sqlcode = ECPG_PGSQL; /* %.*s is safe here as long as sqlstate is all-ASCII */ - ecpg_log("raising sqlstate %.*s (sqlcode %d): %s\n", + ecpg_log("raising sqlstate %.*s (sqlcode %ld): %s\n", sizeof(sqlca->sqlstate), sqlca->sqlstate, sqlca->sqlcode, sqlca->sqlerrm.sqlerrmc); /* free all memory we have allocated for the user */ diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c index 2fb54392c9e..a288b3ff125 100644 --- a/src/interfaces/ecpg/ecpglib/execute.c +++ b/src/interfaces/ecpg/ecpglib/execute.c @@ -332,7 +332,7 @@ ecpg_store_result(const PGresult *results, int act_field, */ if ((var->arrsize > 0 && ntuples > var->arrsize) || (var->ind_arrsize > 0 && ntuples > var->ind_arrsize)) { - ecpg_log("ecpg_store_result on line %d: incorrect number of matches; %d don't fit into array of %d\n", + ecpg_log("ecpg_store_result on line %d: incorrect number of matches; %d don't fit into array of %ld\n", stmt->lineno, ntuples, var->arrsize); ecpg_raise(stmt->lineno, INFORMIX_MODE(stmt->compat) ? ECPG_INFORMIX_SUBSELECT_NOT_ONE : ECPG_TOO_MANY_MATCHES, ECPG_SQLSTATE_CARDINALITY_VIOLATION, NULL); return false; diff --git a/src/interfaces/ecpg/ecpglib/extern.h b/src/interfaces/ecpg/ecpglib/extern.h index 0d55102d0da..96d49a4a6b7 100644 --- a/src/interfaces/ecpg/ecpglib/extern.h +++ b/src/interfaces/ecpg/ecpglib/extern.h @@ -161,7 +161,7 @@ void ecpg_raise(int line, int code, const char *sqlstate, const char *str); void ecpg_raise_backend(int line, PGresult *result, PGconn *conn, int compat); char *ecpg_prepared(const char *, struct connection *); bool ecpg_deallocate_all_conn(int lineno, enum COMPAT_MODE c, struct connection * conn); -void ecpg_log(const char *format,...); +void ecpg_log(const char *format, ...) __attribute__((format(PG_PRINTF_ATTRIBUTE, 1, 2))); bool ecpg_auto_prepare(int, const char *, const int, char **, const char *); void ecpg_init_sqlca(struct sqlca_t * sqlca);