On 2014-04-02 23:50:19 +0200, Andres Freund wrote:
> > > I just tried it on clang. It builds clean with -Wc11-extensions except
> > > warning about _Static_assert(). That's possibly fixable with some
> > > autoconf trickery.
> >
> > Ah. That sounds promising. What clang version is that?
>
> It's debian's clang-3.5, which is built from trunk IIUC: 1:3.5~svn201651-1
>
> I have some patches that fix the configure tests to properly use
> -Werror, but I am too tired to check their validity now.
So, three patches attached:
1) fix a couple of warnings clang outputs in -pedantic. All of them
somewhat valid and not too ugly to fix.
2) Use -Werror in a couple more configure checks. That allows to compile
pg using both -Wc11-extensions and -Wc99-extensions in a halfway sane
fashion. I think this is sane, but I'd welcome comments.
3) Fix C89 compliance issue in pgbench, plus minor cleanups. I don't
like the fix here, but I don't really have a better idea.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
>From 3219b14205f33e4e6c3682cf9ca795159d59e611 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Thu, 3 Apr 2014 12:51:40 +0200
Subject: [PATCH 1/3] Fix a bunch of somewhat pedantic compiler warnings.
In C89 it's not allowed to have a trailing comma after the last
enumerator value, compound literals have to be compile time constant
and strictly speaking a void * pointer isn't a function pointer...
---
src/backend/access/transam/xlog.c | 2 +-
src/bin/pg_dump/parallel.c | 5 ++++-
src/bin/psql/tab-complete.c | 10 +++++-----
src/include/catalog/objectaccess.h | 2 +-
src/include/pgstat.h | 2 +-
src/include/utils/jsonapi.h | 2 +-
6 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index f9d6609..5096999 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -651,7 +651,7 @@ typedef enum
XLOG_FROM_ANY = 0, /* request to read WAL from any source */
XLOG_FROM_ARCHIVE, /* restored using restore_command */
XLOG_FROM_PG_XLOG, /* existing file in pg_xlog */
- XLOG_FROM_STREAM, /* streamed from master */
+ XLOG_FROM_STREAM /* streamed from master */
} XLogSource;
/* human-readable names for XLogSources, for debugging output */
diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c
index 6f2634b..7d6e146 100644
--- a/src/bin/pg_dump/parallel.c
+++ b/src/bin/pg_dump/parallel.c
@@ -558,7 +558,10 @@ ParallelBackupStart(ArchiveHandle *AH, RestoreOptions *ropt)
{
/* we are the worker */
int j;
- int pipefd[2] = {pipeMW[PIPE_READ], pipeWM[PIPE_WRITE]};
+ int pipefd[2];
+
+ pipefd[0] = pipeMW[PIPE_READ];
+ pipefd[1] = pipeWM[PIPE_WRITE];
/*
* Store the fds for the reverse communication in pstate. Actually
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 1d69b95..202ffce 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -781,7 +781,7 @@ static const pgsql_thing_t words_after_create[] = {
/* Forward declaration of functions */
-static char **psql_completion(char *text, int start, int end);
+static char **psql_completion(const char *text, int start, int end);
static char *create_command_generator(const char *text, int state);
static char *drop_command_generator(const char *text, int state);
static char *complete_from_query(const char *text, int state);
@@ -790,7 +790,7 @@ static char *_complete_from_query(int is_schema_query,
const char *text, int state);
static char *complete_from_list(const char *text, int state);
static char *complete_from_const(const char *text, int state);
-static char **complete_from_variables(char *text,
+static char **complete_from_variables(const char *text,
const char *prefix, const char *suffix);
static char *complete_from_files(const char *text, int state);
@@ -812,7 +812,7 @@ void
initialize_readline(void)
{
rl_readline_name = (char *) pset.progname;
- rl_attempted_completion_function = (void *) psql_completion;
+ rl_attempted_completion_function = psql_completion;
rl_basic_word_break_characters = WORD_BREAKS;
@@ -834,7 +834,7 @@ initialize_readline(void)
* completion_matches() function, so we don't have to worry about it.
*/
static char **
-psql_completion(char *text, int start, int end)
+psql_completion(const char *text, int start, int end)
{
/* This is the variable we'll return. */
char **matches = NULL;
@@ -3847,7 +3847,7 @@ complete_from_const(const char *text, int state)
* to support quoting usages.
*/
static char **
-complete_from_variables(char *text, const char *prefix, const char *suffix)
+complete_from_variables(const char *text, const char *prefix, const char *suffix)
{
char **matches;
char **varnames;
diff --git a/src/include/catalog/objectaccess.h b/src/include/catalog/objectaccess.h
index e2f6b14..ac8260b 100644
--- a/src/include/catalog/objectaccess.h
+++ b/src/include/catalog/objectaccess.h
@@ -45,7 +45,7 @@ typedef enum ObjectAccessType
OAT_DROP,
OAT_POST_ALTER,
OAT_NAMESPACE_SEARCH,
- OAT_FUNCTION_EXECUTE,
+ OAT_FUNCTION_EXECUTE
} ObjectAccessType;
/*
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 932c83d..5f131f8 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -676,7 +676,7 @@ typedef enum BackendState
STATE_IDLEINTRANSACTION,
STATE_FASTPATH,
STATE_IDLEINTRANSACTION_ABORTED,
- STATE_DISABLED,
+ STATE_DISABLED
} BackendState;
/* ----------
diff --git a/src/include/utils/jsonapi.h b/src/include/utils/jsonapi.h
index 7a4fbfe..e4a2bd5 100644
--- a/src/include/utils/jsonapi.h
+++ b/src/include/utils/jsonapi.h
@@ -30,7 +30,7 @@ typedef enum
JSON_TOKEN_TRUE,
JSON_TOKEN_FALSE,
JSON_TOKEN_NULL,
- JSON_TOKEN_END,
+ JSON_TOKEN_END
} JsonTokenType;
--
1.8.5.rc2.dirty
>From 06b20b158ced12812586d94205609435ef29e3db Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Thu, 3 Apr 2014 17:12:39 +0200
Subject: [PATCH 2/3] Fix configure to test more thoroughly for the compiler's
capabilities.
The old version of the _Static_assert(), variadic macros,
FLEXIBLE_ARRAY_MEMBER tests succeeded, even if the compiler warned
about the usage of the functionality. That lead to very noisy builds
for compilers instructed to warn about nonstandard functionality being
used.
---
config/c-compiler.m4 | 28 +++++++++++++++++++++++++---
configure | 15 +++++++++++++--
configure.in | 2 +-
3 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/config/c-compiler.m4 b/config/c-compiler.m4
index 4ba3236..5c59d37 100644
--- a/config/c-compiler.m4
+++ b/config/c-compiler.m4
@@ -130,14 +130,18 @@ fi])# PGAC_C_FUNCNAME_SUPPORT
# gcc-style compound expressions to be able to wrap the thing into macros.
AC_DEFUN([PGAC_C_STATIC_ASSERT],
[AC_CACHE_CHECK(for _Static_assert, pgac_cv__static_assert,
-[AC_TRY_LINK([],
+[ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
+AC_TRY_LINK([],
[({ _Static_assert(1, "foo"); })],
[pgac_cv__static_assert=yes],
[pgac_cv__static_assert=no])])
if test x"$pgac_cv__static_assert" = xyes ; then
AC_DEFINE(HAVE__STATIC_ASSERT, 1,
[Define to 1 if your compiler understands _Static_assert.])
-fi])# PGAC_C_STATIC_ASSERT
+fi
+ac_c_werror_flag=$ac_save_c_werror_flag
+])# PGAC_C_STATIC_ASSERT
@@ -205,12 +209,15 @@ fi])# PGAC_C_BUILTIN_UNREACHABLE
# and define HAVE__VA_ARGS if so.
AC_DEFUN([PGAC_C_VA_ARGS],
[AC_CACHE_CHECK(for __VA_ARGS__, pgac_cv__va_args,
-[AC_TRY_COMPILE([#include <stdio.h>],
+[ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
+AC_TRY_COMPILE([#include <stdio.h>],
[#define debug(...) fprintf(stderr, __VA_ARGS__)
debug("%s", "blarg");
],
[pgac_cv__va_args=yes],
[pgac_cv__va_args=no])])
+ac_c_werror_flag=$ac_save_c_werror_flag
if test x"$pgac_cv__va_args" = xyes ; then
AC_DEFINE(HAVE__VA_ARGS, 1,
[Define to 1 if your compiler understands __VA_ARGS__ in macros.])
@@ -289,3 +296,18 @@ if test x"$Ac_cachevar" = x"yes"; then
fi
undefine([Ac_cachevar])dnl
])# PGAC_PROG_CC_LDFLAGS_OPT
+
+
+# PGAC_C_FLEXIBLE_ARRAY_MEMBER
+# ------------------------
+# Check whether the compiler supports flexible array members. In
+# contrast to the autoconf version we don't accept any warnings
+# because using flexible array members in compilers that warn about
+# them will just result in a very noisy compile.
+AC_DEFUN([PGAC_C_FLEXIBLE_ARRAY_MEMBER],
+[ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
+dnl Just use the autoconf test after forcing -Werror
+AC_C_FLEXIBLE_ARRAY_MEMBER()
+ac_c_werror_flag=$ac_save_c_werror_flag
+])# PGAC_C_FLEXIBLE_ARRAY_MEMBER
diff --git a/configure b/configure
index 122ace7..1f3ed0f 100755
--- a/configure
+++ b/configure
@@ -9751,6 +9751,8 @@ _ACEOF
fi
+ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for flexible array members" >&5
$as_echo_n "checking for flexible array members... " >&6; }
@@ -9793,6 +9795,8 @@ $as_echo "#define FLEXIBLE_ARRAY_MEMBER /**/" >>confdefs.h
fi
+ac_c_werror_flag=$ac_save_c_werror_flag
+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for signed types" >&5
$as_echo_n "checking for signed types... " >&6; }
if ${pgac_cv_c_signed+:} false; then :
@@ -9889,7 +9893,9 @@ $as_echo_n "checking for _Static_assert... " >&6; }
if ${pgac_cv__static_assert+:} false; then :
$as_echo_n "(cached) " >&6
else
- cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+ ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
int
@@ -9915,6 +9921,8 @@ if test x"$pgac_cv__static_assert" = xyes ; then
$as_echo "#define HAVE__STATIC_ASSERT 1" >>confdefs.h
fi
+ac_c_werror_flag=$ac_save_c_werror_flag
+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_types_compatible_p" >&5
$as_echo_n "checking for __builtin_types_compatible_p... " >&6; }
if ${pgac_cv__types_compatible+:} false; then :
@@ -10011,7 +10019,9 @@ $as_echo_n "checking for __VA_ARGS__... " >&6; }
if ${pgac_cv__va_args+:} false; then :
$as_echo_n "(cached) " >&6
else
- cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+ ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
#include <stdio.h>
int
@@ -10033,6 +10043,7 @@ rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
fi
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__va_args" >&5
$as_echo "$pgac_cv__va_args" >&6; }
+ac_c_werror_flag=$ac_save_c_werror_flag
if test x"$pgac_cv__va_args" = xyes ; then
$as_echo "#define HAVE__VA_ARGS 1" >>confdefs.h
diff --git a/configure.in b/configure.in
index 0f3e0cc..d1651f2 100644
--- a/configure.in
+++ b/configure.in
@@ -1092,7 +1092,7 @@ fi
m4_defun([AC_PROG_CC_STDC], []) dnl We don't want that.
AC_C_BIGENDIAN
PGAC_C_INLINE
-AC_C_FLEXIBLE_ARRAY_MEMBER
+PGAC_C_FLEXIBLE_ARRAY_MEMBER
PGAC_C_SIGNED
PGAC_C_FUNCNAME_SUPPORT
PGAC_C_STATIC_ASSERT
--
1.8.5.rc2.dirty
>From 6549005c3c762bbad2e89be0816b6852579dae77 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Thu, 3 Apr 2014 17:09:43 +0200
Subject: [PATCH 3/3] Make pgbench more C89 compliant in a slightly ugly way.
C89 requires that compound literals are constant, but pgbench had them
dependant on the passed in scale.
Also cleanup confusion of function level usage of static vs. const.
---
contrib/pgbench/pgbench.c | 45 ++++++++++++++++++++++++++++++++++-----------
1 file changed, 34 insertions(+), 11 deletions(-)
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 7c1e59e..406160d 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -1566,12 +1566,10 @@ init(bool is_no_vacuum)
char *cols;
int declare_fillfactor;
};
- struct ddlinfo DDLs[] = {
+ const struct ddlinfo DDLs_int[] = {
{
"pgbench_history",
- scale >= SCALE_32BIT_THRESHOLD
- ? "tid int,bid int,aid bigint,delta int,mtime timestamp,filler char(22)"
- : "tid int,bid int,aid int,delta int,mtime timestamp,filler char(22)",
+ "tid int,bid int,aid int,delta int,mtime timestamp,filler char(22)",
0
},
{
@@ -1581,9 +1579,7 @@ init(bool is_no_vacuum)
},
{
"pgbench_accounts",
- scale >= SCALE_32BIT_THRESHOLD
- ? "aid bigint not null,bid int,abalance int,filler char(84)"
- : "aid int not null,bid int,abalance int,filler char(84)",
+ "aid int not null,bid int,abalance int,filler char(84)",
1
},
{
@@ -1592,12 +1588,34 @@ init(bool is_no_vacuum)
1
}
};
- static char *DDLAFTERs[] = {
+ const struct ddlinfo DDLs_bigint[] = {
+ {
+ "pgbench_history",
+ "tid int,bid int,aid bigint,delta int,mtime timestamp,filler char(22)",
+ 0
+ },
+ {
+ "pgbench_tellers",
+ "tid int not null,bid int,tbalance int,filler char(84)",
+ 1
+ },
+ {
+ "pgbench_accounts",
+ "aid bigint not null,bid int,abalance int,filler char(84)",
+ 1
+ },
+ {
+ "pgbench_branches",
+ "bid int not null,bbalance int,filler char(88)",
+ 1
+ }
+ };
+ const char * const DDLAFTERs[] = {
"alter table pgbench_branches add primary key (bid)",
"alter table pgbench_tellers add primary key (tid)",
"alter table pgbench_accounts add primary key (aid)"
};
- static char *DDLKEYs[] = {
+ const char * const DDLKEYs[] = {
"alter table pgbench_tellers add foreign key (bid) references pgbench_branches",
"alter table pgbench_accounts add foreign key (bid) references pgbench_branches",
"alter table pgbench_history add foreign key (bid) references pgbench_branches",
@@ -1621,11 +1639,16 @@ init(bool is_no_vacuum)
if ((con = doConnect()) == NULL)
exit(1);
- for (i = 0; i < lengthof(DDLs); i++)
+ for (i = 0; i < lengthof(DDLs_bigint); i++)
{
char opts[256];
char buffer[256];
- struct ddlinfo *ddl = &DDLs[i];
+ const struct ddlinfo *ddl;
+
+ if (scale >= SCALE_32BIT_THRESHOLD)
+ ddl = &DDLs_bigint[i];
+ else
+ ddl = &DDLs_int[i];
/* Remove old table, if it exists. */
snprintf(buffer, 256, "drop table if exists %s", ddl->table);
--
1.8.5.rc2.dirty
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
