public inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH] Add -Wold-style-definition to CFLAGS.
27+ messages / 3 participants
[nested] [flat]

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* [PATCH] Add -Wold-style-definition to CFLAGS.
@ 2020-05-09 17:38  Andres Freund <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Andres Freund @ 2020-05-09 17:38 UTC (permalink / raw)

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


--zqfhnontb2zb7pps--





^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: "38.10.10. Shared Memory and LWLocks" may require a clarification
@ 2023-05-21 10:10  Aleksander Alekseev <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Aleksander Alekseev @ 2023-05-21 10:10 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>

Hi,

> However it's not clear when a race-condition may happen. The rest of
> the text gives an overall impression that the shmem_startup_hook will
> be called by postmaster once (unless an extension places several hooks
> in series). Thus there is no real need to ackquire AddinShmemInitLock
> and it should be safe to store LWLock* in local process memory. This
> memory will be inherited from postmaster by child processes and the
> overall memory usage is going to be the same due to copy-on-write.

I added some logs and comments to my toy extension [1] to demonstrate
this. Additionally I added a sleep() call to the shmem_startup_hook to
make sure there are no concurrent processes at the moment when the
hook is called (this change is not committed to the GitHub
repository):

```
@@ -35,6 +35,9 @@ experiment_shmem_request(void)
        RequestNamedLWLockTranche("experiment", 1);
 }

+#include <stdio.h>
+#include <unistd.h>
+
 static void
 experiment_shmem_startup(void)
 {
@@ -43,6 +46,8 @@ experiment_shmem_startup(void)
        elog(LOG, "experiment_shmem_startup(): pid = %d, postmaster = %d\n",
                MyProcPid, !IsUnderPostmaster);

+    sleep(30);
+
        if(prev_shmem_startup_hook)
                prev_shmem_startup_hook();
```

If we do `make && make install && make installcheck` and examine
.//tmp_check/log/001_basic_main.log we will see:

```
[6288] LOG:  _PG_init(): pid = 6288, postmaster = 1
[6288] LOG:  experiment_shmem_request(): pid = 6288, postmaster = 1
[6288] LOG:  experiment_shmem_startup(): pid = 6288, postmaster = 1
```

Also we can make sure that there is only one process running when
shmem_startup_hook is called.

So it looks like acquiring AddinShmemInitLock in the hook is redundant
and also placing LWLock* in local process memory instead of shared
memory is safe.

Unless I missed something, I suggest updating the documentation and
pg_stat_statements.c accordingly.

[1]: https://github.com/afiskon/postgresql-extensions/blob/main/006-shared-memory/experiment.c#L38

-- 
Best regards,
Aleksander Alekseev






^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: "38.10.10. Shared Memory and LWLocks" may require a clarification
@ 2023-05-22 13:19  Aleksander Alekseev <[email protected]>
  parent: Aleksander Alekseev <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Aleksander Alekseev @ 2023-05-22 13:19 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>

Hi,

> Unless I missed something, I suggest updating the documentation and
> pg_stat_statements.c accordingly.

Since no one seems to object so far I prepared the patch.

-- 
Best regards,
Aleksander Alekseev


Attachments:

  [application/octet-stream] v1-0001-Clarify-the-usage-of-shmem_startup_hook.patch (15.6K, ../../CAJ7c6TP9rpCujvvnCHwrgUnQeqJ=8NaD0aag-oeFWcg=OZB6zA@mail.gmail.com/2-v1-0001-Clarify-the-usage-of-shmem_startup_hook.patch)
  download | inline diff:
From e1d383a40fe4b502dc5284293f37749e1e670bd2 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Sun, 21 May 2023 16:47:09 +0300
Subject: [PATCH v1] Clarify the usage of shmem_startup_hook

Prior to this patch an example of requesting shared memory and LWLocks from
an extension provided in pg_stat_statements.c acquired AddinShmemInitLock and
placed a pointer to LWLocks tranche in a shared memory.

This is redundant since the extension uses shmem_startup_hook which is executed
by postmaster before fork()'ing any child processes. Although it works this may
confuse the reader and make one think that acquiring the lock etc is absolutely
necessary.

Refactor pg_stat_statements.c accordingly and clarify the usage of
shmem_startup_hook in the documentation.

Aleksander Alekseev, reviewed by TODO FIXME
Discussion: https://postgr.es/m/CAJ7c6TPKhFgL%2B54cdTD9yGpG4%2BsNcyJ%2BN1GvQqAxgWENAOa3VA%40mail.gmail.com
---
 .../pg_stat_statements/pg_stat_statements.c   | 116 +++++++++---------
 doc/src/sgml/xfunc.sgml                       |  30 +++--
 src/backend/storage/ipc/ipci.c                |   7 ++
 src/backend/utils/init/miscinit.c             |   6 +
 4 files changed, 90 insertions(+), 69 deletions(-)

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 68215bb2e3..e867cc6170 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -20,17 +20,17 @@
  * file are kept in shared memory.
  *
  * Note about locking issues: to create or delete an entry in the shared
- * hashtable, one must hold pgss->lock exclusively.  Modifying any field
+ * hashtable, one must hold pgss_lock exclusively.  Modifying any field
  * in an entry except the counters requires the same.  To look up an entry,
  * one must hold the lock shared.  To read or update the counters within
  * an entry, one must hold the lock shared or exclusive (so the entry doesn't
  * disappear!) and also take the entry's mutex spinlock.
  * The shared state variable pgss->extent (the next free spot in the external
  * query-text file) should be accessed only while holding either the
- * pgss->mutex spinlock, or exclusive lock on pgss->lock.  We use the mutex to
- * allow reserving file space while holding only shared lock on pgss->lock.
+ * pgss->mutex spinlock, or exclusive lock on pgss_lock.  We use the mutex to
+ * allow reserving file space while holding only shared lock on pgss_lock.
  * Rewriting the entire external query-text file, eg for garbage collection,
- * requires holding pgss->lock exclusively; this allows individual entries
+ * requires holding pgss_lock exclusively; this allows individual entries
  * in the file to be read or written while holding only shared lock.
  *
  *
@@ -228,12 +228,22 @@ typedef struct pgssEntry
 	slock_t		mutex;			/* protects the counters only */
 } pgssEntry;
 
+
+/*
+ * pgss_lock protects hashtable search/modification
+ *
+ * The value is initialized in shmem_startup_hook. The hook is guaranteed
+ * to be called from the postmaster before it fork()'s any other processes.
+ * Child processes are going to inherit pgss_lock value. For this reason it's
+ * not necessary (although possible) to store this value in shared memory.
+ */
+LWLock	   *pgss_lock = NULL;
+
 /*
  * Global shared state
  */
 typedef struct pgssSharedState
 {
-	LWLock	   *lock;			/* protects hashtable search/modification */
 	double		cur_median_usage;	/* current median usage in hashtable */
 	Size		mean_query_len; /* current mean entry text length */
 	slock_t		mutex;			/* protects following fields only: */
@@ -494,6 +504,10 @@ pgss_shmem_request(void)
  * then load any pre-existing statistics from file.
  * Also create and load the query-texts file, which is expected to exist
  * (even if empty) while the module is enabled.
+ *
+ * Note: we don't bother with locks here, because there should be no other
+ * processes running when this code is reached. The hook is guaranteed to be
+ * called by the postmaster before any other processes are going to be started.
  */
 static void
 pgss_shmem_startup(void)
@@ -509,6 +523,8 @@ pgss_shmem_startup(void)
 	int			buffer_size;
 	char	   *buffer = NULL;
 
+	Assert(!IsUnderPostmaster);
+
 	if (prev_shmem_startup_hook)
 		prev_shmem_startup_hook();
 
@@ -516,28 +532,21 @@ pgss_shmem_startup(void)
 	pgss = NULL;
 	pgss_hash = NULL;
 
-	/*
-	 * Create or attach to the shared memory state, including hash table
-	 */
-	LWLockAcquire(AddinShmemInitLock, LW_EXCLUSIVE);
-
 	pgss = ShmemInitStruct("pg_stat_statements",
 						   sizeof(pgssSharedState),
 						   &found);
 
-	if (!found)
-	{
-		/* First time through ... */
-		pgss->lock = &(GetNamedLWLockTranche("pg_stat_statements"))->lock;
-		pgss->cur_median_usage = ASSUMED_MEDIAN_INIT;
-		pgss->mean_query_len = ASSUMED_LENGTH_INIT;
-		SpinLockInit(&pgss->mutex);
-		pgss->extent = 0;
-		pgss->n_writers = 0;
-		pgss->gc_count = 0;
-		pgss->stats.dealloc = 0;
-		pgss->stats.stats_reset = GetCurrentTimestamp();
-	}
+	Assert(!found);
+
+	pgss_lock = &(GetNamedLWLockTranche("pg_stat_statements"))->lock;
+	pgss->cur_median_usage = ASSUMED_MEDIAN_INIT;
+	pgss->mean_query_len = ASSUMED_LENGTH_INIT;
+	SpinLockInit(&pgss->mutex);
+	pgss->extent = 0;
+	pgss->n_writers = 0;
+	pgss->gc_count = 0;
+	pgss->stats.dealloc = 0;
+	pgss->stats.stats_reset = GetCurrentTimestamp();
 
 	info.keysize = sizeof(pgssHashKey);
 	info.entrysize = sizeof(pgssEntry);
@@ -546,25 +555,10 @@ pgss_shmem_startup(void)
 							  &info,
 							  HASH_ELEM | HASH_BLOBS);
 
-	LWLockRelease(AddinShmemInitLock);
-
-	/*
-	 * If we're in the postmaster (or a standalone backend...), set up a shmem
-	 * exit hook to dump the statistics to disk.
-	 */
-	if (!IsUnderPostmaster)
-		on_shmem_exit(pgss_shmem_shutdown, (Datum) 0);
-
-	/*
-	 * Done if some other process already completed our initialization.
-	 */
-	if (found)
-		return;
-
 	/*
-	 * Note: we don't bother with locks here, because there should be no other
-	 * processes running when this code is reached.
+	 * Set up a shmem exit hook to dump the statistics to disk.
 	 */
+	on_shmem_exit(pgss_shmem_shutdown, (Datum) 0);
 
 	/* Unlink query text file possibly left over from crash */
 	unlink(PGSS_TEXT_FILE);
@@ -1266,7 +1260,7 @@ pgss_store(const char *query, uint64 queryId,
 	key.toplevel = (exec_nested_level == 0);
 
 	/* Lookup the hash table entry with shared lock. */
-	LWLockAcquire(pgss->lock, LW_SHARED);
+	LWLockAcquire(pgss_lock, LW_SHARED);
 
 	entry = (pgssEntry *) hash_search(pgss_hash, &key, HASH_FIND, NULL);
 
@@ -1287,11 +1281,11 @@ pgss_store(const char *query, uint64 queryId,
 		 */
 		if (jstate)
 		{
-			LWLockRelease(pgss->lock);
+			LWLockRelease(pgss_lock);
 			norm_query = generate_normalized_query(jstate, query,
 												   query_location,
 												   &query_len);
-			LWLockAcquire(pgss->lock, LW_SHARED);
+			LWLockAcquire(pgss_lock, LW_SHARED);
 		}
 
 		/* Append new query text to file with only shared lock held */
@@ -1306,8 +1300,8 @@ pgss_store(const char *query, uint64 queryId,
 		do_gc = need_gc_qtexts();
 
 		/* Need exclusive lock to make a new hashtable entry - promote */
-		LWLockRelease(pgss->lock);
-		LWLockAcquire(pgss->lock, LW_EXCLUSIVE);
+		LWLockRelease(pgss_lock);
+		LWLockAcquire(pgss_lock, LW_EXCLUSIVE);
 
 		/*
 		 * A garbage collection may have occurred while we weren't holding the
@@ -1419,7 +1413,7 @@ pgss_store(const char *query, uint64 queryId,
 	}
 
 done:
-	LWLockRelease(pgss->lock);
+	LWLockRelease(pgss_lock);
 
 	/* We postpone this clean-up until we're out of the lock */
 	if (norm_query)
@@ -1612,7 +1606,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
 
 	/*
 	 * We'd like to load the query text file (if needed) while not holding any
-	 * lock on pgss->lock.  In the worst case we'll have to do this again
+	 * lock on pgss_lock.  In the worst case we'll have to do this again
 	 * after we have the lock, but it's unlikely enough to make this a win
 	 * despite occasional duplicated work.  We need to reload if anybody
 	 * writes to the file (either a retail qtext_store(), or a garbage
@@ -1651,7 +1645,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
 	 * we need to partition the hash table to limit the time spent holding any
 	 * one lock.
 	 */
-	LWLockAcquire(pgss->lock, LW_SHARED);
+	LWLockAcquire(pgss_lock, LW_SHARED);
 
 	if (showtext)
 	{
@@ -1851,7 +1845,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
 		tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, values, nulls);
 	}
 
-	LWLockRelease(pgss->lock);
+	LWLockRelease(pgss_lock);
 
 	free(qbuffer);
 }
@@ -1910,7 +1904,7 @@ pgss_memsize(void)
 
 /*
  * Allocate a new hashtable entry.
- * caller must hold an exclusive lock on pgss->lock
+ * caller must hold an exclusive lock on pgss_lock
  *
  * "query" need not be null-terminated; we rely on query_len instead
  *
@@ -1979,7 +1973,7 @@ entry_cmp(const void *lhs, const void *rhs)
 /*
  * Deallocate least-used entries.
  *
- * Caller must hold an exclusive lock on pgss->lock.
+ * Caller must hold an exclusive lock on pgss_lock.
  */
 static void
 entry_dealloc(void)
@@ -2070,7 +2064,7 @@ entry_dealloc(void)
  *
  * On failure, returns false.
  *
- * At least a shared lock on pgss->lock must be held by the caller, so as
+ * At least a shared lock on pgss_lock must be held by the caller, so as
  * to prevent a concurrent garbage collection.  Share-lock-holding callers
  * should pass a gc_count pointer to obtain the number of garbage collections,
  * so that they can recheck the count after obtaining exclusive lock to
@@ -2165,7 +2159,7 @@ error:
  *
  * On success, the buffer size is also returned into *buffer_size.
  *
- * This can be called without any lock on pgss->lock, but in that case
+ * This can be called without any lock on pgss_lock, but in that case
  * the caller is responsible for verifying that the result is sane.
  */
 static char *
@@ -2282,7 +2276,7 @@ qtext_fetch(Size query_offset, int query_len,
 /*
  * Do we need to garbage-collect the external query text file?
  *
- * Caller should hold at least a shared lock on pgss->lock.
+ * Caller should hold at least a shared lock on pgss_lock.
  */
 static bool
 need_gc_qtexts(void)
@@ -2331,7 +2325,7 @@ need_gc_qtexts(void)
  * becomes unreasonably large, with no other method of compaction likely to
  * occur in the foreseeable future.
  *
- * The caller must hold an exclusive lock on pgss->lock.
+ * The caller must hold an exclusive lock on pgss_lock.
  *
  * At the first sign of trouble we unlink the query text file to get a clean
  * slate (although existing statistics are retained), rather than risk
@@ -2458,7 +2452,7 @@ gc_qtexts(void)
 
 	/*
 	 * OK, count a garbage collection cycle.  (Note: even though we have
-	 * exclusive lock on pgss->lock, we must take pgss->mutex for this, since
+	 * exclusive lock on pgss_lock, we must take pgss->mutex for this, since
 	 * other processes may examine gc_count while holding only the mutex.
 	 * Also, we have to advance the count *after* we've rewritten the file,
 	 * else other processes might not realize they read a stale file.)
@@ -2507,12 +2501,12 @@ gc_fail:
 	 * Bump the GC count even though we failed.
 	 *
 	 * This is needed to make concurrent readers of file without any lock on
-	 * pgss->lock notice existence of new version of file.  Once readers
-	 * subsequently observe a change in GC count with pgss->lock held, that
+	 * pgss_lock notice existence of new version of file.  Once readers
+	 * subsequently observe a change in GC count with pgss_lock held, that
 	 * forces a safe reopen of file.  Writers also require that we bump here,
 	 * of course.  (As required by locking protocol, readers and writers don't
 	 * trust earlier file contents until gc_count is found unchanged after
-	 * pgss->lock acquired in shared or exclusive mode respectively.)
+	 * pgss_lock acquired in shared or exclusive mode respectively.)
 	 */
 	record_gc_qtexts();
 }
@@ -2535,7 +2529,7 @@ entry_reset(Oid userid, Oid dbid, uint64 queryid)
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 				 errmsg("pg_stat_statements must be loaded via shared_preload_libraries")));
 
-	LWLockAcquire(pgss->lock, LW_EXCLUSIVE);
+	LWLockAcquire(pgss_lock, LW_EXCLUSIVE);
 	num_entries = hash_get_num_entries(pgss_hash);
 
 	if (userid != 0 && dbid != 0 && queryid != UINT64CONST(0))
@@ -2633,7 +2627,7 @@ done:
 	record_gc_qtexts();
 
 release_lock:
-	LWLockRelease(pgss->lock);
+	LWLockRelease(pgss_lock);
 }
 
 /*
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 9620ea9ae3..4fa8c1f52c 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3424,14 +3424,9 @@ void RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks)
      to get a pointer to this array.
     </para>
     <para>
-     An example of a <literal>shmem_request_hook</literal> can be found in
-     <filename>contrib/pg_stat_statements/pg_stat_statements.c</filename> in the
-     <productname>PostgreSQL</productname> source tree.
-    </para>
-    <para>
-     To avoid possible race-conditions, each backend should use the LWLock
-     <function>AddinShmemInitLock</function> when connecting to and initializing
-     its allocation of shared memory, as shown here:
+     Generally, to avoid possible race-conditions, each backend should use the
+     LWLock <function>AddinShmemInitLock</function> when connecting to and
+     initializing its allocation of shared memory, as shown here:
 <programlisting>
 static mystruct *ptr = NULL;
 
@@ -3451,6 +3446,25 @@ if (!ptr)
 }
 </programlisting>
     </para>
+    <para>
+    Alternatively an extension can use <literal>shmem_startup_hook</literal>.
+    This allows to place all the code responsible for initializing shared memory
+    in a single place. Additionally the hook is guaranteed to be called by
+    postmaster before creating any other child processes, similarly to
+    <function>_PG_init</function> and <literal>shmem_request_hook</literal>.
+    </para>
+    <para>
+    When using <literal>shmem_startup_hook</literal> there is no need to acquire
+    <function>AddinShmemInitLock</function>. Also in this case a pointer to
+    LWLocks tranche returned by <function>GetNamedLWLockTranche</function> can
+    be stored in local process memory. The memory is going to be inherited from
+    postmaster by the child processes.
+    </para>
+    <para>
+     An complete example of allocating shared memory and LWLocks can be found in
+     <filename>contrib/pg_stat_statements/pg_stat_statements.c</filename> in the
+     <productname>PostgreSQL</productname> source tree.
+    </para>
    </sect2>
 
    <sect2 id="extend-cpp">
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c
index 8f1ded7338..715b2dab80 100644
--- a/src/backend/storage/ipc/ipci.c
+++ b/src/backend/storage/ipc/ipci.c
@@ -312,7 +312,14 @@ CreateSharedMemoryAndSemaphores(void)
 	 * Now give loadable modules a chance to set up their shmem allocations
 	 */
 	if (shmem_startup_hook)
+	{
+		/*
+		 * The following assert ensures that shmem_startup_hook is going to be
+		 * called only by the postmaster, as promised in the documentation.
+		 */
+		Assert(!IsUnderPostmaster);
 		shmem_startup_hook();
+	}
 }
 
 /*
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index a604432126..933132f438 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -1874,6 +1874,12 @@ process_session_preload_libraries(void)
 void
 process_shmem_requests(void)
 {
+	/*
+	 * The following assert ensures that shmem_request_hook is going to be
+	 * called only by the postmaster, as promised in the documentation.
+	 */
+	Assert(!IsUnderPostmaster);
+
 	process_shmem_requests_in_progress = true;
 	if (shmem_request_hook)
 		shmem_request_hook();
-- 
2.40.1



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: "38.10.10. Shared Memory and LWLocks" may require a clarification
@ 2023-05-22 14:04  Aleksander Alekseev <[email protected]>
  parent: Aleksander Alekseev <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Aleksander Alekseev @ 2023-05-22 14:04 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>

Hi,

> Since no one seems to object so far I prepared the patch.

Turned out patch v1 fails on cfbot on Windows due to extra Assert I added [1]:

```
abort() has been calledTRAP: failed Assert("!IsUnderPostmaster"),
File: "../src/backend/storage/ipc/ipci.c", Line: 320, PID: 4040
abort() has been calledTRAP: failed Assert("!IsUnderPostmaster"),
File: "../src/backend/storage/ipc/ipci.c", Line: 320, PID: 3484
```

Which indicates that currently shmem_startup_hook **can** be called by
child processes on Windows. Not 100% sure if this is a desired
behavior considering the fact that it is inconsistent with the current
behavior on *nix systems.

Here is patch v2. Changes comparing to v1:

```
--- a/src/backend/storage/ipc/ipci.c
+++ b/src/backend/storage/ipc/ipci.c
@@ -311,15 +311,8 @@ CreateSharedMemoryAndSemaphores(void)
        /*
         * Now give loadable modules a chance to set up their shmem allocations
         */
-       if (shmem_startup_hook)
-       {
-               /*
-                * The following assert ensures that
shmem_startup_hook is going to be
-                * called only by the postmaster, as promised in the
documentation.
-                */
-               Assert(!IsUnderPostmaster);
+       if (!IsUnderPostmaster && shmem_startup_hook)
                shmem_startup_hook();
-       }
 }
```

Thoughts?

[1]: https://api.cirrus-ci.com/v1/artifact/task/4924036300406784/testrun/build/testrun/pg_stat_statements...

-- 
Best regards,
Aleksander Alekseev


Attachments:

  [application/octet-stream] v2-0001-Clarify-the-usage-of-shmem_startup_hook.patch (15.5K, ../../CAJ7c6TNyZiqWv4We76FjNPQkwtna65yKwaVErwWjv+JyLEnbpA@mail.gmail.com/2-v2-0001-Clarify-the-usage-of-shmem_startup_hook.patch)
  download | inline diff:
From 47b157717b2f6e6a0894c9d83fbbc831957ed2a5 Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Sun, 21 May 2023 16:47:09 +0300
Subject: [PATCH v2] Clarify the usage of shmem_startup_hook

Prior to this patch an example of requesting shared memory and LWLocks from
an extension provided in pg_stat_statements.c acquired AddinShmemInitLock and
placed a pointer to LWLocks tranche in a shared memory.

This is redundant since the extension uses shmem_startup_hook which is executed
by postmaster before fork()'ing any child processes. Although it works this may
confuse the reader and make one think that acquiring the lock etc is absolutely
necessary.

Refactor pg_stat_statements.c accordingly and clarify the usage of
shmem_startup_hook in the documentation.

Aleksander Alekseev, reviewed by TODO FIXME
Discussion: https://postgr.es/m/CAJ7c6TPKhFgL%2B54cdTD9yGpG4%2BsNcyJ%2BN1GvQqAxgWENAOa3VA%40mail.gmail.com
---
 .../pg_stat_statements/pg_stat_statements.c   | 116 +++++++++---------
 doc/src/sgml/xfunc.sgml                       |  30 +++--
 src/backend/storage/ipc/ipci.c                |   2 +-
 src/backend/utils/init/miscinit.c             |   6 +
 4 files changed, 84 insertions(+), 70 deletions(-)

diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 68215bb2e3..e867cc6170 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -20,17 +20,17 @@
  * file are kept in shared memory.
  *
  * Note about locking issues: to create or delete an entry in the shared
- * hashtable, one must hold pgss->lock exclusively.  Modifying any field
+ * hashtable, one must hold pgss_lock exclusively.  Modifying any field
  * in an entry except the counters requires the same.  To look up an entry,
  * one must hold the lock shared.  To read or update the counters within
  * an entry, one must hold the lock shared or exclusive (so the entry doesn't
  * disappear!) and also take the entry's mutex spinlock.
  * The shared state variable pgss->extent (the next free spot in the external
  * query-text file) should be accessed only while holding either the
- * pgss->mutex spinlock, or exclusive lock on pgss->lock.  We use the mutex to
- * allow reserving file space while holding only shared lock on pgss->lock.
+ * pgss->mutex spinlock, or exclusive lock on pgss_lock.  We use the mutex to
+ * allow reserving file space while holding only shared lock on pgss_lock.
  * Rewriting the entire external query-text file, eg for garbage collection,
- * requires holding pgss->lock exclusively; this allows individual entries
+ * requires holding pgss_lock exclusively; this allows individual entries
  * in the file to be read or written while holding only shared lock.
  *
  *
@@ -228,12 +228,22 @@ typedef struct pgssEntry
 	slock_t		mutex;			/* protects the counters only */
 } pgssEntry;
 
+
+/*
+ * pgss_lock protects hashtable search/modification
+ *
+ * The value is initialized in shmem_startup_hook. The hook is guaranteed
+ * to be called from the postmaster before it fork()'s any other processes.
+ * Child processes are going to inherit pgss_lock value. For this reason it's
+ * not necessary (although possible) to store this value in shared memory.
+ */
+LWLock	   *pgss_lock = NULL;
+
 /*
  * Global shared state
  */
 typedef struct pgssSharedState
 {
-	LWLock	   *lock;			/* protects hashtable search/modification */
 	double		cur_median_usage;	/* current median usage in hashtable */
 	Size		mean_query_len; /* current mean entry text length */
 	slock_t		mutex;			/* protects following fields only: */
@@ -494,6 +504,10 @@ pgss_shmem_request(void)
  * then load any pre-existing statistics from file.
  * Also create and load the query-texts file, which is expected to exist
  * (even if empty) while the module is enabled.
+ *
+ * Note: we don't bother with locks here, because there should be no other
+ * processes running when this code is reached. The hook is guaranteed to be
+ * called by the postmaster before any other processes are going to be started.
  */
 static void
 pgss_shmem_startup(void)
@@ -509,6 +523,8 @@ pgss_shmem_startup(void)
 	int			buffer_size;
 	char	   *buffer = NULL;
 
+	Assert(!IsUnderPostmaster);
+
 	if (prev_shmem_startup_hook)
 		prev_shmem_startup_hook();
 
@@ -516,28 +532,21 @@ pgss_shmem_startup(void)
 	pgss = NULL;
 	pgss_hash = NULL;
 
-	/*
-	 * Create or attach to the shared memory state, including hash table
-	 */
-	LWLockAcquire(AddinShmemInitLock, LW_EXCLUSIVE);
-
 	pgss = ShmemInitStruct("pg_stat_statements",
 						   sizeof(pgssSharedState),
 						   &found);
 
-	if (!found)
-	{
-		/* First time through ... */
-		pgss->lock = &(GetNamedLWLockTranche("pg_stat_statements"))->lock;
-		pgss->cur_median_usage = ASSUMED_MEDIAN_INIT;
-		pgss->mean_query_len = ASSUMED_LENGTH_INIT;
-		SpinLockInit(&pgss->mutex);
-		pgss->extent = 0;
-		pgss->n_writers = 0;
-		pgss->gc_count = 0;
-		pgss->stats.dealloc = 0;
-		pgss->stats.stats_reset = GetCurrentTimestamp();
-	}
+	Assert(!found);
+
+	pgss_lock = &(GetNamedLWLockTranche("pg_stat_statements"))->lock;
+	pgss->cur_median_usage = ASSUMED_MEDIAN_INIT;
+	pgss->mean_query_len = ASSUMED_LENGTH_INIT;
+	SpinLockInit(&pgss->mutex);
+	pgss->extent = 0;
+	pgss->n_writers = 0;
+	pgss->gc_count = 0;
+	pgss->stats.dealloc = 0;
+	pgss->stats.stats_reset = GetCurrentTimestamp();
 
 	info.keysize = sizeof(pgssHashKey);
 	info.entrysize = sizeof(pgssEntry);
@@ -546,25 +555,10 @@ pgss_shmem_startup(void)
 							  &info,
 							  HASH_ELEM | HASH_BLOBS);
 
-	LWLockRelease(AddinShmemInitLock);
-
-	/*
-	 * If we're in the postmaster (or a standalone backend...), set up a shmem
-	 * exit hook to dump the statistics to disk.
-	 */
-	if (!IsUnderPostmaster)
-		on_shmem_exit(pgss_shmem_shutdown, (Datum) 0);
-
-	/*
-	 * Done if some other process already completed our initialization.
-	 */
-	if (found)
-		return;
-
 	/*
-	 * Note: we don't bother with locks here, because there should be no other
-	 * processes running when this code is reached.
+	 * Set up a shmem exit hook to dump the statistics to disk.
 	 */
+	on_shmem_exit(pgss_shmem_shutdown, (Datum) 0);
 
 	/* Unlink query text file possibly left over from crash */
 	unlink(PGSS_TEXT_FILE);
@@ -1266,7 +1260,7 @@ pgss_store(const char *query, uint64 queryId,
 	key.toplevel = (exec_nested_level == 0);
 
 	/* Lookup the hash table entry with shared lock. */
-	LWLockAcquire(pgss->lock, LW_SHARED);
+	LWLockAcquire(pgss_lock, LW_SHARED);
 
 	entry = (pgssEntry *) hash_search(pgss_hash, &key, HASH_FIND, NULL);
 
@@ -1287,11 +1281,11 @@ pgss_store(const char *query, uint64 queryId,
 		 */
 		if (jstate)
 		{
-			LWLockRelease(pgss->lock);
+			LWLockRelease(pgss_lock);
 			norm_query = generate_normalized_query(jstate, query,
 												   query_location,
 												   &query_len);
-			LWLockAcquire(pgss->lock, LW_SHARED);
+			LWLockAcquire(pgss_lock, LW_SHARED);
 		}
 
 		/* Append new query text to file with only shared lock held */
@@ -1306,8 +1300,8 @@ pgss_store(const char *query, uint64 queryId,
 		do_gc = need_gc_qtexts();
 
 		/* Need exclusive lock to make a new hashtable entry - promote */
-		LWLockRelease(pgss->lock);
-		LWLockAcquire(pgss->lock, LW_EXCLUSIVE);
+		LWLockRelease(pgss_lock);
+		LWLockAcquire(pgss_lock, LW_EXCLUSIVE);
 
 		/*
 		 * A garbage collection may have occurred while we weren't holding the
@@ -1419,7 +1413,7 @@ pgss_store(const char *query, uint64 queryId,
 	}
 
 done:
-	LWLockRelease(pgss->lock);
+	LWLockRelease(pgss_lock);
 
 	/* We postpone this clean-up until we're out of the lock */
 	if (norm_query)
@@ -1612,7 +1606,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
 
 	/*
 	 * We'd like to load the query text file (if needed) while not holding any
-	 * lock on pgss->lock.  In the worst case we'll have to do this again
+	 * lock on pgss_lock.  In the worst case we'll have to do this again
 	 * after we have the lock, but it's unlikely enough to make this a win
 	 * despite occasional duplicated work.  We need to reload if anybody
 	 * writes to the file (either a retail qtext_store(), or a garbage
@@ -1651,7 +1645,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
 	 * we need to partition the hash table to limit the time spent holding any
 	 * one lock.
 	 */
-	LWLockAcquire(pgss->lock, LW_SHARED);
+	LWLockAcquire(pgss_lock, LW_SHARED);
 
 	if (showtext)
 	{
@@ -1851,7 +1845,7 @@ pg_stat_statements_internal(FunctionCallInfo fcinfo,
 		tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, values, nulls);
 	}
 
-	LWLockRelease(pgss->lock);
+	LWLockRelease(pgss_lock);
 
 	free(qbuffer);
 }
@@ -1910,7 +1904,7 @@ pgss_memsize(void)
 
 /*
  * Allocate a new hashtable entry.
- * caller must hold an exclusive lock on pgss->lock
+ * caller must hold an exclusive lock on pgss_lock
  *
  * "query" need not be null-terminated; we rely on query_len instead
  *
@@ -1979,7 +1973,7 @@ entry_cmp(const void *lhs, const void *rhs)
 /*
  * Deallocate least-used entries.
  *
- * Caller must hold an exclusive lock on pgss->lock.
+ * Caller must hold an exclusive lock on pgss_lock.
  */
 static void
 entry_dealloc(void)
@@ -2070,7 +2064,7 @@ entry_dealloc(void)
  *
  * On failure, returns false.
  *
- * At least a shared lock on pgss->lock must be held by the caller, so as
+ * At least a shared lock on pgss_lock must be held by the caller, so as
  * to prevent a concurrent garbage collection.  Share-lock-holding callers
  * should pass a gc_count pointer to obtain the number of garbage collections,
  * so that they can recheck the count after obtaining exclusive lock to
@@ -2165,7 +2159,7 @@ error:
  *
  * On success, the buffer size is also returned into *buffer_size.
  *
- * This can be called without any lock on pgss->lock, but in that case
+ * This can be called without any lock on pgss_lock, but in that case
  * the caller is responsible for verifying that the result is sane.
  */
 static char *
@@ -2282,7 +2276,7 @@ qtext_fetch(Size query_offset, int query_len,
 /*
  * Do we need to garbage-collect the external query text file?
  *
- * Caller should hold at least a shared lock on pgss->lock.
+ * Caller should hold at least a shared lock on pgss_lock.
  */
 static bool
 need_gc_qtexts(void)
@@ -2331,7 +2325,7 @@ need_gc_qtexts(void)
  * becomes unreasonably large, with no other method of compaction likely to
  * occur in the foreseeable future.
  *
- * The caller must hold an exclusive lock on pgss->lock.
+ * The caller must hold an exclusive lock on pgss_lock.
  *
  * At the first sign of trouble we unlink the query text file to get a clean
  * slate (although existing statistics are retained), rather than risk
@@ -2458,7 +2452,7 @@ gc_qtexts(void)
 
 	/*
 	 * OK, count a garbage collection cycle.  (Note: even though we have
-	 * exclusive lock on pgss->lock, we must take pgss->mutex for this, since
+	 * exclusive lock on pgss_lock, we must take pgss->mutex for this, since
 	 * other processes may examine gc_count while holding only the mutex.
 	 * Also, we have to advance the count *after* we've rewritten the file,
 	 * else other processes might not realize they read a stale file.)
@@ -2507,12 +2501,12 @@ gc_fail:
 	 * Bump the GC count even though we failed.
 	 *
 	 * This is needed to make concurrent readers of file without any lock on
-	 * pgss->lock notice existence of new version of file.  Once readers
-	 * subsequently observe a change in GC count with pgss->lock held, that
+	 * pgss_lock notice existence of new version of file.  Once readers
+	 * subsequently observe a change in GC count with pgss_lock held, that
 	 * forces a safe reopen of file.  Writers also require that we bump here,
 	 * of course.  (As required by locking protocol, readers and writers don't
 	 * trust earlier file contents until gc_count is found unchanged after
-	 * pgss->lock acquired in shared or exclusive mode respectively.)
+	 * pgss_lock acquired in shared or exclusive mode respectively.)
 	 */
 	record_gc_qtexts();
 }
@@ -2535,7 +2529,7 @@ entry_reset(Oid userid, Oid dbid, uint64 queryid)
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 				 errmsg("pg_stat_statements must be loaded via shared_preload_libraries")));
 
-	LWLockAcquire(pgss->lock, LW_EXCLUSIVE);
+	LWLockAcquire(pgss_lock, LW_EXCLUSIVE);
 	num_entries = hash_get_num_entries(pgss_hash);
 
 	if (userid != 0 && dbid != 0 && queryid != UINT64CONST(0))
@@ -2633,7 +2627,7 @@ done:
 	record_gc_qtexts();
 
 release_lock:
-	LWLockRelease(pgss->lock);
+	LWLockRelease(pgss_lock);
 }
 
 /*
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 9620ea9ae3..4fa8c1f52c 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3424,14 +3424,9 @@ void RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks)
      to get a pointer to this array.
     </para>
     <para>
-     An example of a <literal>shmem_request_hook</literal> can be found in
-     <filename>contrib/pg_stat_statements/pg_stat_statements.c</filename> in the
-     <productname>PostgreSQL</productname> source tree.
-    </para>
-    <para>
-     To avoid possible race-conditions, each backend should use the LWLock
-     <function>AddinShmemInitLock</function> when connecting to and initializing
-     its allocation of shared memory, as shown here:
+     Generally, to avoid possible race-conditions, each backend should use the
+     LWLock <function>AddinShmemInitLock</function> when connecting to and
+     initializing its allocation of shared memory, as shown here:
 <programlisting>
 static mystruct *ptr = NULL;
 
@@ -3451,6 +3446,25 @@ if (!ptr)
 }
 </programlisting>
     </para>
+    <para>
+    Alternatively an extension can use <literal>shmem_startup_hook</literal>.
+    This allows to place all the code responsible for initializing shared memory
+    in a single place. Additionally the hook is guaranteed to be called by
+    postmaster before creating any other child processes, similarly to
+    <function>_PG_init</function> and <literal>shmem_request_hook</literal>.
+    </para>
+    <para>
+    When using <literal>shmem_startup_hook</literal> there is no need to acquire
+    <function>AddinShmemInitLock</function>. Also in this case a pointer to
+    LWLocks tranche returned by <function>GetNamedLWLockTranche</function> can
+    be stored in local process memory. The memory is going to be inherited from
+    postmaster by the child processes.
+    </para>
+    <para>
+     An complete example of allocating shared memory and LWLocks can be found in
+     <filename>contrib/pg_stat_statements/pg_stat_statements.c</filename> in the
+     <productname>PostgreSQL</productname> source tree.
+    </para>
    </sect2>
 
    <sect2 id="extend-cpp">
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c
index 8f1ded7338..912048068e 100644
--- a/src/backend/storage/ipc/ipci.c
+++ b/src/backend/storage/ipc/ipci.c
@@ -311,7 +311,7 @@ CreateSharedMemoryAndSemaphores(void)
 	/*
 	 * Now give loadable modules a chance to set up their shmem allocations
 	 */
-	if (shmem_startup_hook)
+	if (!IsUnderPostmaster && shmem_startup_hook)
 		shmem_startup_hook();
 }
 
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index a604432126..933132f438 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -1874,6 +1874,12 @@ process_session_preload_libraries(void)
 void
 process_shmem_requests(void)
 {
+	/*
+	 * The following assert ensures that shmem_request_hook is going to be
+	 * called only by the postmaster, as promised in the documentation.
+	 */
+	Assert(!IsUnderPostmaster);
+
 	process_shmem_requests_in_progress = true;
 	if (shmem_request_hook)
 		shmem_request_hook();
-- 
2.40.1



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: "38.10.10. Shared Memory and LWLocks" may require a clarification
@ 2023-05-23 10:47  Aleksander Alekseev <[email protected]>
  parent: Aleksander Alekseev <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Aleksander Alekseev @ 2023-05-23 10:47 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>

Hi hackers,

That's me still talking to myself :)

> Thoughts?

Evidently this works differently from what I initially thought on
Windows due to lack of fork() on this system.

PFA the patch v3. Your feedback is most welcomed.

--
Best regards,
Aleksander Alekseev


Attachments:

  [application/octet-stream] v3-0001-Clarify-the-38.10.10.-Shared-Memory-and-LWLocks-s.patch (2.6K, ../../CAJ7c6TMeU7wati28umoMBCB=YTpdkqqtxuj4yGVT=_=kOaqmow@mail.gmail.com/2-v3-0001-Clarify-the-38.10.10.-Shared-Memory-and-LWLocks-s.patch)
  download | inline diff:
From 6bc126ccec7baabc7319fc5fd276531ba5d8a43a Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Tue, 23 May 2023 13:32:40 +0300
Subject: [PATCH v3] Clarify the "38.10.10. Shared Memory and LWLocks" section

Prior to this commit the documentation showed an example of acquiring shared
memory and LWLocks from an extension without much clarification why the
extension needs to acquire AddinShmemInitLock. This can puzzle the reader
versed in *nix systems but not particularly mindful of Windows specifics.

Clarify this and also document the existence shmem_startup_hook.

Aleksander Alekseev, reviewed by TODO FIXME
Discussion: https://postgr.es/m/CAJ7c6TPKhFgL%2B54cdTD9yGpG4%2BsNcyJ%2BN1GvQqAxgWENAOa3VA%40mail.gmail.com
---
 doc/src/sgml/xfunc.sgml | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 9620ea9ae3..cf31c114bf 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3423,11 +3423,6 @@ void RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks)
      <literal>tranche_name</literal>.  Use <function>GetNamedLWLockTranche</function>
      to get a pointer to this array.
     </para>
-    <para>
-     An example of a <literal>shmem_request_hook</literal> can be found in
-     <filename>contrib/pg_stat_statements/pg_stat_statements.c</filename> in the
-     <productname>PostgreSQL</productname> source tree.
-    </para>
     <para>
      To avoid possible race-conditions, each backend should use the LWLock
      <function>AddinShmemInitLock</function> when connecting to and initializing
@@ -3451,6 +3446,20 @@ if (!ptr)
 }
 </programlisting>
     </para>
+    <para>
+    It is convenient to use <literal>shmem_startup_hook</literal> which allows
+    placing all the code responsible for initializing shared memory in one place.
+    When using <literal>shmem_startup_hook</literal> the extension still needs
+    to acquire <function>AddinShmemInitLock</function> in order to work properly
+    on all the supported platforms including Windows. This is also the reason
+    why the return value of <function>GetNamedLWLockTranche</function> is
+    conventionally stored in shared memory instead of local process memory.
+    </para>
+    <para>
+     An complete example of allocating shared memory and LWLocks can be found in
+     <filename>contrib/pg_stat_statements/pg_stat_statements.c</filename> in the
+     <productname>PostgreSQL</productname> source tree.
+    </para>
    </sect2>
 
    <sect2 id="extend-cpp">
-- 
2.40.1



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: "38.10.10. Shared Memory and LWLocks" may require a clarification
@ 2023-10-27 06:52  Michael Paquier <[email protected]>
  parent: Aleksander Alekseev <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Michael Paquier @ 2023-10-27 06:52 UTC (permalink / raw)
  To: Aleksander Alekseev <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

On Tue, May 23, 2023 at 01:47:52PM +0300, Aleksander Alekseev wrote:
> That's me still talking to myself :)

Let's be two then.

> Evidently this works differently from what I initially thought on
> Windows due to lack of fork() on this system.

This comes down to the fact that processes executed with EXEC_BACKEND,
because Windows does not know how to do a fork(), need to update their
local variables to point to the shared memory structures already
created, so we have to call CreateSharedMemoryAndSemaphores() in this
case.

> PFA the patch v3. Your feedback is most welcomed.

+    <para>
+    It is convenient to use <literal>shmem_startup_hook</literal> which allows
+    placing all the code responsible for initializing shared memory in one place.
+    When using <literal>shmem_startup_hook</literal> the extension still needs
+    to acquire <function>AddinShmemInitLock</function> in order to work properly
+    on all the supported platforms including Windows.

Yeah, AddinShmemInitLock is useful because extensions have no base
point outside that, and they may want to update their local variables.
Still, this is not completely exact because EXEC_BACKEND on
non-Windows platform would still need it, so this should be mentioned.
Another thing is that extensions may do like autoprewarm.c, where
the shmem area is not initialized in the startup shmem hook.  This is
a bit cheating because this is a scenario where shmem_request_hook is
not requested, so shmem needs overallocation, but you'd also need a
LWLock in this case, even for non-WIN32.

+    on all the supported platforms including Windows. This is also the reason
+    why the return value of <function>GetNamedLWLockTranche</function> is
+    conventionally stored in shared memory instead of local process memory.
+    </para>

Not sure to follow this sentence, the result of GetNamedLWLockTranche
is the lock, so this sentence does not seem really necessary?

While we're on it, why not improving this part of the documentation more
modern?  We don't mention LWLockNewTrancheId() and
LWLockRegisterTranche() at all, so do you think that it would be worth
adding a sample of code with that, mentioning autoprewarm.c as
example?
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: "38.10.10. Shared Memory and LWLocks" may require a clarification
@ 2023-10-30 13:03  Aleksander Alekseev <[email protected]>
  parent: Michael Paquier <[email protected]>
  0 siblings, 1 reply; 27+ messages in thread

From: Aleksander Alekseev @ 2023-10-30 13:03 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>; +Cc: Michael Paquier <[email protected]>

Michael,

On Fri, Oct 27, 2023 at 9:52 AM Michael Paquier <[email protected]> wrote:
>
> On Tue, May 23, 2023 at 01:47:52PM +0300, Aleksander Alekseev wrote:
> > That's me still talking to myself :)
>
> Let's be two then.

Many thanks for your feedback.

> +    <para>
> +    It is convenient to use <literal>shmem_startup_hook</literal> which allows
> +    placing all the code responsible for initializing shared memory in one place.
> +    When using <literal>shmem_startup_hook</literal> the extension still needs
> +    to acquire <function>AddinShmemInitLock</function> in order to work properly
> +    on all the supported platforms including Windows.
>
> Yeah, AddinShmemInitLock is useful because extensions have no base
> point outside that, and they may want to update their local variables.
> Still, this is not completely exact because EXEC_BACKEND on
> non-Windows platform would still need it, so this should be mentioned.
> Another thing is that extensions may do like autoprewarm.c, where
> the shmem area is not initialized in the startup shmem hook.  This is
> a bit cheating because this is a scenario where shmem_request_hook is
> not requested, so shmem needs overallocation, but you'd also need a
> LWLock in this case, even for non-WIN32.

Got it. Let's simply remove the "including Windows" part then.

>
> +    on all the supported platforms including Windows. This is also the reason
> +    why the return value of <function>GetNamedLWLockTranche</function> is
> +    conventionally stored in shared memory instead of local process memory.
> +    </para>
>
> Not sure to follow this sentence, the result of GetNamedLWLockTranche
> is the lock, so this sentence does not seem really necessary?

To be honest, by now I don't remember what was meant here, so I
removed the sentence.

> While we're on it, why not improving this part of the documentation more
> modern?  We don't mention LWLockNewTrancheId() and
> LWLockRegisterTranche() at all, so do you think that it would be worth
> adding a sample of code with that, mentioning autoprewarm.c as
> example?

Agree, these functions deserve to be mentioned in this section.

PFA patch v4.

-- 
Best regards,
Aleksander Alekseev


Attachments:

  [application/octet-stream] v4-0001-Clarify-the-38.10.10.-Shared-Memory-and-LWLocks-s.patch (2.8K, ../../CAJ7c6TMC2TWeez++so3QuLjW0rzjt-Z7KZtyaYSfAWSM7wa8=w@mail.gmail.com/2-v4-0001-Clarify-the-38.10.10.-Shared-Memory-and-LWLocks-s.patch)
  download | inline diff:
From 892c661f4ced5ce47a713031987cbc153a98798c Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Tue, 23 May 2023 13:32:40 +0300
Subject: [PATCH v4] Clarify the "38.10.10. Shared Memory and LWLocks" section

Prior to this commit the documentation showed an example of acquiring shared
memory and LWLocks from an extension without much clarification why the
extension needs to acquire AddinShmemInitLock. Clarify this and also document
the existence shmem_startup_hook.

While on it also mention the existence of LWLockNewTrancheId() and
LWLockRegisterTranche().

Aleksander Alekseev, reviewed by Michael Paquier

Discussion: https://postgr.es/m/CAJ7c6TPKhFgL%2B54cdTD9yGpG4%2BsNcyJ%2BN1GvQqAxgWENAOa3VA%40mail.gmail.com
---
 doc/src/sgml/xfunc.sgml | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 96ba95818c..2884ca4742 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3424,9 +3424,18 @@ void RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks)
      to get a pointer to this array.
     </para>
     <para>
-     An example of a <literal>shmem_request_hook</literal> can be found in
-     <filename>contrib/pg_stat_statements/pg_stat_statements.c</filename> in the
-     <productname>PostgreSQL</productname> source tree.
+     When necessary, an unique tranche ID can be allocated:
+<programlisting>
+int LWLockNewTrancheId(void)
+</programlisting>
+     Also a dynamic name in the lookup table of the current process can be assigned to the tranche:
+<programlisting>
+void LWLockRegisterTranche(int tranche_id, const char *tranche_name)
+</programlisting>
+    A complete usage example of <function>LWLockNewTrancheId</function> and
+    <function>LWLockRegisterTranche</function> can be found in
+    <filename>contrib/pg_prewarm/autoprewarm.c</filename> in the
+    <productname>PostgreSQL</productname> source tree.
     </para>
     <para>
      To avoid possible race-conditions, each backend should use the LWLock
@@ -3451,6 +3460,18 @@ if (!ptr)
 }
 </programlisting>
     </para>
+    <para>
+    It is convenient to use <literal>shmem_startup_hook</literal> which allows
+    placing all the code responsible for initializing shared memory in one place.
+    When using <literal>shmem_startup_hook</literal> the extension still needs
+    to acquire <function>AddinShmemInitLock</function> in order to work properly
+    on all the supported platforms.
+    </para>
+    <para>
+     A complete example of allocating shared memory and LWLocks can be found in
+     <filename>contrib/pg_stat_statements/pg_stat_statements.c</filename> in the
+     <productname>PostgreSQL</productname> source tree.
+    </para>
    </sect2>
 
    <sect2 id="xfunc-addin-wait-events">
-- 
2.42.0



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

* Re: "38.10.10. Shared Memory and LWLocks" may require a clarification
@ 2023-10-30 13:10  Aleksander Alekseev <[email protected]>
  parent: Aleksander Alekseev <[email protected]>
  0 siblings, 0 replies; 27+ messages in thread

From: Aleksander Alekseev @ 2023-10-30 13:10 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>; +Cc: Michael Paquier <[email protected]>

Hi,

> PFA patch v4.

I didn't like the commit message. Here is the corrected patch. Sorry
for the noise.

-- 
Best regards,
Aleksander Alekseev


Attachments:

  [application/octet-stream] v5-0001-Clarify-the-38.10.10.-Shared-Memory-and-LWLocks-s.patch (2.5K, ../../CAJ7c6TPqU5VMvCv75=wN0axyc3XpaquRar1bbBehV=3Eob192g@mail.gmail.com/2-v5-0001-Clarify-the-38.10.10.-Shared-Memory-and-LWLocks-s.patch)
  download | inline diff:
From 43c986a7859d0e270f630bab7a333f99e5d5da8e Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <[email protected]>
Date: Tue, 23 May 2023 13:32:40 +0300
Subject: [PATCH v5] Clarify the "38.10.10. Shared Memory and LWLocks" section

Document the existence shmem_startup_hook, LWLockNewTrancheId() and
LWLockRegisterTranche().

Aleksander Alekseev, reviewed by Michael Paquier

Discussion: https://postgr.es/m/CAJ7c6TPKhFgL%2B54cdTD9yGpG4%2BsNcyJ%2BN1GvQqAxgWENAOa3VA%40mail.gmail.com
---
 doc/src/sgml/xfunc.sgml | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 96ba95818c..2884ca4742 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3424,9 +3424,18 @@ void RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks)
      to get a pointer to this array.
     </para>
     <para>
-     An example of a <literal>shmem_request_hook</literal> can be found in
-     <filename>contrib/pg_stat_statements/pg_stat_statements.c</filename> in the
-     <productname>PostgreSQL</productname> source tree.
+     When necessary, an unique tranche ID can be allocated:
+<programlisting>
+int LWLockNewTrancheId(void)
+</programlisting>
+     Also a dynamic name in the lookup table of the current process can be assigned to the tranche:
+<programlisting>
+void LWLockRegisterTranche(int tranche_id, const char *tranche_name)
+</programlisting>
+    A complete usage example of <function>LWLockNewTrancheId</function> and
+    <function>LWLockRegisterTranche</function> can be found in
+    <filename>contrib/pg_prewarm/autoprewarm.c</filename> in the
+    <productname>PostgreSQL</productname> source tree.
     </para>
     <para>
      To avoid possible race-conditions, each backend should use the LWLock
@@ -3451,6 +3460,18 @@ if (!ptr)
 }
 </programlisting>
     </para>
+    <para>
+    It is convenient to use <literal>shmem_startup_hook</literal> which allows
+    placing all the code responsible for initializing shared memory in one place.
+    When using <literal>shmem_startup_hook</literal> the extension still needs
+    to acquire <function>AddinShmemInitLock</function> in order to work properly
+    on all the supported platforms.
+    </para>
+    <para>
+     A complete example of allocating shared memory and LWLocks can be found in
+     <filename>contrib/pg_stat_statements/pg_stat_statements.c</filename> in the
+     <productname>PostgreSQL</productname> source tree.
+    </para>
    </sect2>
 
    <sect2 id="xfunc-addin-wait-events">
-- 
2.42.0



^ permalink  raw  reply  [nested|flat] 27+ messages in thread

end of thread, other threads:[~2023-10-30 13:10 UTC | newest]

Thread overview: 27+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2020-05-09 17:38 [PATCH] Add -Wold-style-definition to CFLAGS. Andres Freund <[email protected]>
2023-05-21 10:10 Re: "38.10.10. Shared Memory and LWLocks" may require a clarification Aleksander Alekseev <[email protected]>
2023-05-22 13:19 ` Re: "38.10.10. Shared Memory and LWLocks" may require a clarification Aleksander Alekseev <[email protected]>
2023-05-22 14:04   ` Re: "38.10.10. Shared Memory and LWLocks" may require a clarification Aleksander Alekseev <[email protected]>
2023-05-23 10:47     ` Re: "38.10.10. Shared Memory and LWLocks" may require a clarification Aleksander Alekseev <[email protected]>
2023-10-27 06:52       ` Re: "38.10.10. Shared Memory and LWLocks" may require a clarification Michael Paquier <[email protected]>
2023-10-30 13:03         ` Re: "38.10.10. Shared Memory and LWLocks" may require a clarification Aleksander Alekseev <[email protected]>
2023-10-30 13:10           ` Re: "38.10.10. Shared Memory and LWLocks" may require a clarification Aleksander Alekseev <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox