Hi,

ISTM that it's our coding style that we use

something
my_paramless_func(void)
{
...
}

definitions rather than omitting the (void), which makes the function
look like an old-style function declaration. I somewhat regularly notice
such omissions during review, and fix them.

Since gcc has a warning detecting such definition, I think we ought to
automatically add it when available?

The attached patch makes configure do so, and also fixes a handful of
uses that crept in.

Greetings,

Andres Freund
>From 21ed43b0b0b57c29e31973d1fea0e3ed2f93a4d4 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Sat, 9 May 2020 10:38:02 -0700
Subject: [PATCH] Add -Wold-style-definition to CFLAGS.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/postmaster/autovacuum.c       |  2 +-
 src/backend/storage/buffer/freelist.c     |  2 +-
 src/backend/utils/adt/jsonpath_scan.l     |  2 +-
 src/backend/utils/misc/queryenvironment.c |  2 +-
 src/interfaces/libpq/fe-misc.c            |  2 +-
 configure                                 | 41 +++++++++++++++++++++++
 configure.in                              |  2 ++
 7 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 7e97ffab27d..5641f59016d 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -834,7 +834,7 @@ HandleAutoVacLauncherInterrupts(void)
  * Perform a normal exit from the autovac launcher.
  */
 static void
-AutoVacLauncherShutdown()
+AutoVacLauncherShutdown(void)
 {
 	ereport(DEBUG1,
 			(errmsg("autovacuum launcher shutting down")));
diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index aa5539a2e45..942f8d4edd2 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -177,7 +177,7 @@ ClockSweepTick(void)
  * should not call this.
  */
 bool
-have_free_buffer()
+have_free_buffer(void)
 {
 	if (StrategyControl->firstFreeBuffer >= 0)
 		return true;
diff --git a/src/backend/utils/adt/jsonpath_scan.l b/src/backend/utils/adt/jsonpath_scan.l
index be0a2cfa2f7..df56a268d62 100644
--- a/src/backend/utils/adt/jsonpath_scan.l
+++ b/src/backend/utils/adt/jsonpath_scan.l
@@ -330,7 +330,7 @@ static const JsonPathKeyword keywords[] = {
 
 /* Check if current scanstring value is a keyword */
 static enum yytokentype
-checkKeyword()
+checkKeyword(void)
 {
 	int						res = IDENT_P;
 	int						diff;
diff --git a/src/backend/utils/misc/queryenvironment.c b/src/backend/utils/misc/queryenvironment.c
index c0b85ed0b36..31de81f353e 100644
--- a/src/backend/utils/misc/queryenvironment.c
+++ b/src/backend/utils/misc/queryenvironment.c
@@ -36,7 +36,7 @@ struct QueryEnvironment
 
 
 QueryEnvironment *
-create_queryEnv()
+create_queryEnv(void)
 {
 	return (QueryEnvironment *) palloc0(sizeof(QueryEnvironment));
 }
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 19729f96311..9afa0533a62 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1250,7 +1250,7 @@ PQenv2encoding(void)
 #ifdef ENABLE_NLS
 
 static void
-libpq_binddomain()
+libpq_binddomain(void)
 {
 	static bool already_bound = false;
 
diff --git a/configure b/configure
index 83abe872aa6..b5bd3b2e9c0 100755
--- a/configure
+++ b/configure
@@ -5321,6 +5321,47 @@ if test x"$pgac_cv_prog_CC_cflags__Wdeclaration_after_statement" = x"yes"; then
 fi
 
 
+  # Not applicable for C++
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wold-style-definition, for CFLAGS" >&5
+$as_echo_n "checking whether ${CC} supports -Wold-style-definition, for CFLAGS... " >&6; }
+if ${pgac_cv_prog_CC_cflags__Wold_style_definition+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  pgac_save_CFLAGS=$CFLAGS
+pgac_save_CC=$CC
+CC=${CC}
+CFLAGS="${CFLAGS} -Wold-style-definition"
+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
+main ()
+{
+
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+  pgac_cv_prog_CC_cflags__Wold_style_definition=yes
+else
+  pgac_cv_prog_CC_cflags__Wold_style_definition=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"
+CC="$pgac_save_CC"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CC_cflags__Wold_style_definition" >&5
+$as_echo "$pgac_cv_prog_CC_cflags__Wold_style_definition" >&6; }
+if test x"$pgac_cv_prog_CC_cflags__Wold_style_definition" = x"yes"; then
+  CFLAGS="${CFLAGS} -Wold-style-definition"
+fi
+
+
   # -Wdeclaration-after-statement isn't applicable for C++.  Specific C files
   # disable it, so AC_SUBST the negative form.
   PERMIT_DECLARATION_AFTER_STATEMENT=
diff --git a/configure.in b/configure.in
index ecdf1723967..2b0a88ab911 100644
--- a/configure.in
+++ b/configure.in
@@ -482,6 +482,8 @@ if test "$GCC" = yes -a "$ICC" = no; then
   # These work in some but not all gcc versions
   save_CFLAGS=$CFLAGS
   PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement])
+  # Not applicable for C++
+  PGAC_PROG_CC_CFLAGS_OPT([-Wold-style-definition])
   # -Wdeclaration-after-statement isn't applicable for C++.  Specific C files
   # disable it, so AC_SUBST the negative form.
   PERMIT_DECLARATION_AFTER_STATEMENT=
-- 
2.25.0.114.g5b0ca878e0

Reply via email to