pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedAdd -Wold-style-definition to CFLAGS?
9+ messages / 4 participants
[nested] [flat]
* Add -Wold-style-definition to CFLAGS?
@ 2020-05-09 17:48 Andres Freund <andres@anarazel.de>
2020-05-09 18:15 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 9+ messages in thread
From: Andres Freund @ 2020-05-09 17:48 UTC (permalink / raw)
To: pgsql-hackers
Hi,
ISTM that it's our coding style that we use
something
my_paramless_func(void)
{
...
}
definitions rather than omitting the (void), which makes the function
look like an old-style function declaration. I somewhat regularly notice
such omissions during review, and fix them.
Since gcc has a warning detecting such definition, I think we ought to
automatically add it when available?
The attached patch makes configure do so, and also fixes a handful of
uses that crept in.
Greetings,
Andres Freund
Attachments:
0001-Add-Wold-style-definition-to-CFLAGS.patch (text/x-diff, 4.9K) download
From 21ed43b0b0b57c29e31973d1fea0e3ed2f93a4d4 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 9 May 2020 10:38:02 -0700
Subject: [PATCH] Add -Wold-style-definition to CFLAGS.
Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
src/backend/postmaster/autovacuum.c | 2 +-
src/backend/storage/buffer/freelist.c | 2 +-
src/backend/utils/adt/jsonpath_scan.l | 2 +-
src/backend/utils/misc/queryenvironment.c | 2 +-
src/interfaces/libpq/fe-misc.c | 2 +-
configure | 41 +++++++++++++++++++++++
configure.in | 2 ++
7 files changed, 48 insertions(+), 5 deletions(-)
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 7e97ffab27d..5641f59016d 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -834,7 +834,7 @@ HandleAutoVacLauncherInterrupts(void)
* Perform a normal exit from the autovac launcher.
*/
static void
-AutoVacLauncherShutdown()
+AutoVacLauncherShutdown(void)
{
ereport(DEBUG1,
(errmsg("autovacuum launcher shutting down")));
diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index aa5539a2e45..942f8d4edd2 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -177,7 +177,7 @@ ClockSweepTick(void)
* should not call this.
*/
bool
-have_free_buffer()
+have_free_buffer(void)
{
if (StrategyControl->firstFreeBuffer >= 0)
return true;
diff --git a/src/backend/utils/adt/jsonpath_scan.l b/src/backend/utils/adt/jsonpath_scan.l
index be0a2cfa2f7..df56a268d62 100644
--- a/src/backend/utils/adt/jsonpath_scan.l
+++ b/src/backend/utils/adt/jsonpath_scan.l
@@ -330,7 +330,7 @@ static const JsonPathKeyword keywords[] = {
/* Check if current scanstring value is a keyword */
static enum yytokentype
-checkKeyword()
+checkKeyword(void)
{
int res = IDENT_P;
int diff;
diff --git a/src/backend/utils/misc/queryenvironment.c b/src/backend/utils/misc/queryenvironment.c
index c0b85ed0b36..31de81f353e 100644
--- a/src/backend/utils/misc/queryenvironment.c
+++ b/src/backend/utils/misc/queryenvironment.c
@@ -36,7 +36,7 @@ struct QueryEnvironment
QueryEnvironment *
-create_queryEnv()
+create_queryEnv(void)
{
return (QueryEnvironment *) palloc0(sizeof(QueryEnvironment));
}
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 19729f96311..9afa0533a62 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1250,7 +1250,7 @@ PQenv2encoding(void)
#ifdef ENABLE_NLS
static void
-libpq_binddomain()
+libpq_binddomain(void)
{
static bool already_bound = false;
diff --git a/configure b/configure
index 83abe872aa6..b5bd3b2e9c0 100755
--- a/configure
+++ b/configure
@@ -5321,6 +5321,47 @@ if test x"$pgac_cv_prog_CC_cflags__Wdeclaration_after_statement" = x"yes"; then
fi
+ # Not applicable for C++
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wold-style-definition, for CFLAGS" >&5
+$as_echo_n "checking whether ${CC} supports -Wold-style-definition, for CFLAGS... " >&6; }
+if ${pgac_cv_prog_CC_cflags__Wold_style_definition+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CFLAGS=$CFLAGS
+pgac_save_CC=$CC
+CC=${CC}
+CFLAGS="${CFLAGS} -Wold-style-definition"
+ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ pgac_cv_prog_CC_cflags__Wold_style_definition=yes
+else
+ pgac_cv_prog_CC_cflags__Wold_style_definition=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ac_c_werror_flag=$ac_save_c_werror_flag
+CFLAGS="$pgac_save_CFLAGS"
+CC="$pgac_save_CC"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CC_cflags__Wold_style_definition" >&5
+$as_echo "$pgac_cv_prog_CC_cflags__Wold_style_definition" >&6; }
+if test x"$pgac_cv_prog_CC_cflags__Wold_style_definition" = x"yes"; then
+ CFLAGS="${CFLAGS} -Wold-style-definition"
+fi
+
+
# -Wdeclaration-after-statement isn't applicable for C++. Specific C files
# disable it, so AC_SUBST the negative form.
PERMIT_DECLARATION_AFTER_STATEMENT=
diff --git a/configure.in b/configure.in
index ecdf1723967..2b0a88ab911 100644
--- a/configure.in
+++ b/configure.in
@@ -482,6 +482,8 @@ if test "$GCC" = yes -a "$ICC" = no; then
# These work in some but not all gcc versions
save_CFLAGS=$CFLAGS
PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement])
+ # Not applicable for C++
+ PGAC_PROG_CC_CFLAGS_OPT([-Wold-style-definition])
# -Wdeclaration-after-statement isn't applicable for C++. Specific C files
# disable it, so AC_SUBST the negative form.
PERMIT_DECLARATION_AFTER_STATEMENT=
--
2.25.0.114.g5b0ca878e0
* Re: Add -Wold-style-definition to CFLAGS?
2020-05-09 17:48 Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
@ 2020-05-09 18:15 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-05-09 18:43 ` Re: Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 9+ messages in thread
From: Tom Lane @ 2020-05-09 18:15 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
> Since gcc has a warning detecting such definition, I think we ought to
> automatically add it when available?
+1
regards, tom lane
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Add -Wold-style-definition to CFLAGS?
2020-05-09 17:48 Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
2020-05-09 18:15 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-05-09 18:43 ` Andres Freund <andres@anarazel.de>
2020-05-09 23:11 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 9+ messages in thread
From: Andres Freund @ 2020-05-09 18:43 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers
Hi,
On 2020-05-09 14:15:01 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Since gcc has a warning detecting such definition, I think we ought to
> > automatically add it when available?
>
> +1
Any opinion about waiting for branching or not?
- Andres
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Add -Wold-style-definition to CFLAGS?
2020-05-09 17:48 Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
2020-05-09 18:15 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
2020-05-09 18:43 ` Re: Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
@ 2020-05-09 23:11 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-05-10 01:37 ` Re: Add -Wold-style-definition to CFLAGS? Michael Paquier <michael@paquier.xyz>
2020-05-11 03:45 ` Re: Add -Wold-style-definition to CFLAGS? Alvaro Herrera <alvherre@2ndquadrant.com>
2020-06-09 05:47 ` Re: Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
0 siblings, 3 replies; 9+ messages in thread
From: Tom Lane @ 2020-05-09 23:11 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
> On 2020-05-09 14:15:01 -0400, Tom Lane wrote:
>> Andres Freund <andres@anarazel.de> writes:
>>> Since gcc has a warning detecting such definition, I think we ought to
>>> automatically add it when available?
>> +1
> Any opinion about waiting for branching or not?
I'd be OK with pushing it now, but I dunno about other people.
If we do want to push this sort of thing now, the nearby changes
to enable fallthrough warnings should go in too.
regards, tom lane
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Add -Wold-style-definition to CFLAGS?
2020-05-09 17:48 Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
2020-05-09 18:15 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
2020-05-09 18:43 ` Re: Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
2020-05-09 23:11 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-05-10 01:37 ` Michael Paquier <michael@paquier.xyz>
2 siblings, 0 replies; 9+ messages in thread
From: Michael Paquier @ 2020-05-10 01:37 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers
On Sat, May 09, 2020 at 07:11:56PM -0400, Tom Lane wrote:
> I'd be OK with pushing it now, but I dunno about other people.
Sounds like a good idea to me to apply this part now.
> If we do want to push this sort of thing now, the nearby changes
> to enable fallthrough warnings should go in too.
If we do that, merging this second part before beta1 is out looks like
a good compromise to me.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Add -Wold-style-definition to CFLAGS?
2020-05-09 17:48 Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
2020-05-09 18:15 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
2020-05-09 18:43 ` Re: Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
2020-05-09 23:11 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-05-11 03:45 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2 siblings, 0 replies; 9+ messages in thread
From: Alvaro Herrera @ 2020-05-11 03:45 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers
On 2020-May-09, Tom Lane wrote:
> If we do want to push this sort of thing now, the nearby changes
> to enable fallthrough warnings should go in too.
I'll get that sorted out tomorrow.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Add -Wold-style-definition to CFLAGS?
2020-05-09 17:48 Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
2020-05-09 18:15 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
2020-05-09 18:43 ` Re: Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
2020-05-09 23:11 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-06-09 05:47 ` Andres Freund <andres@anarazel.de>
2020-06-09 06:03 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
2 siblings, 1 reply; 9+ messages in thread
From: Andres Freund @ 2020-06-09 05:47 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; Noah Misch <noah@leadboat.com>; +Cc: pgsql-hackers
Hi,
On 2020-05-09 19:11:56 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2020-05-09 14:15:01 -0400, Tom Lane wrote:
> >> Andres Freund <andres@anarazel.de> writes:
> >>> Since gcc has a warning detecting such definition, I think we ought to
> >>> automatically add it when available?
>
> >> +1
>
> > Any opinion about waiting for branching or not?
>
> I'd be OK with pushing it now, but I dunno about other people.
I did run into a small bit of trouble doing so. Those seem to make it a
mistake to target 13.
Unfortunately it turns out that our CFLAG configure tests don't reliably
work with -Wold-style-definition. The problem is that the generated
program contains 'int main() {...}', which obviously is an old-style
definition. Which then causes a warning, which in turn causes the cflag
tests to fail because we run them with ac_c_werror_flag=yes.
There's a pretty easy local fix, which is that we can just use
AC_LANG_SOURCE() instead of AC_LANG_PROGRAM()
PGAC_PROG_VARCC_VARFLAGS_OPT(). There seems to be little reason not to
do so.
But that still leaves us with a lot of unnecessary subsequent warnings
for other tests in config.log. They don't cause problems afaics, as
ac_c_werror_flag=yes isn't widely used, but it's still more noisy than
I'd like. And the likelihood of silent failures down the line seems
higher than I'd like.
Upstream autoconf has fixed this in 2014 (1717921a), but since they've
not bothered to release since then...
The easiest way that I can see to deal with that is to simply redefine
the relevant autoconf macro. For me that solves the vast majority of
these bleats in config.log. That's not particularly pretty, but we have
precedent for it... Since it's just 16 lines, I think we can live with
that?
Comments?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Add -Wold-style-definition to CFLAGS?
2020-05-09 17:48 Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
2020-05-09 18:15 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
2020-05-09 18:43 ` Re: Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
2020-05-09 23:11 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
2020-06-09 05:47 ` Re: Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
@ 2020-06-09 06:03 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-06-09 06:13 ` Re: Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 9+ messages in thread
From: Tom Lane @ 2020-06-09 06:03 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Noah Misch <noah@leadboat.com>; pgsql-hackers
Andres Freund <andres@anarazel.de> writes:
> Unfortunately it turns out that our CFLAG configure tests don't reliably
> work with -Wold-style-definition. The problem is that the generated
> program contains 'int main() {...}', which obviously is an old-style
> definition. Which then causes a warning, which in turn causes the cflag
> tests to fail because we run them with ac_c_werror_flag=yes.
Ugh. I suspect main() might not be the only problem, either.
> Upstream autoconf has fixed this in 2014 (1717921a), but since they've
> not bothered to release since then...
I wonder if there's any way to light a fire under them.
> The easiest way that I can see to deal with that is to simply redefine
> the relevant autoconf macro. For me that solves the vast majority of
> these bleats in config.log. That's not particularly pretty, but we have
> precedent for it... Since it's just 16 lines, I think we can live with
> that?
I don't really think that -Wold-style-definition is worth that.
regards, tom lane
^ permalink raw reply [nested|flat] 9+ messages in thread
* Re: Add -Wold-style-definition to CFLAGS?
2020-05-09 17:48 Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
2020-05-09 18:15 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
2020-05-09 18:43 ` Re: Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
2020-05-09 23:11 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
2020-06-09 05:47 ` Re: Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
2020-06-09 06:03 ` Re: Add -Wold-style-definition to CFLAGS? Tom Lane <tgl@sss.pgh.pa.us>
@ 2020-06-09 06:13 ` Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 9+ messages in thread
From: Andres Freund @ 2020-06-09 06:13 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Noah Misch <noah@leadboat.com>; pgsql-hackers
Hi,
On 2020-06-09 02:03:09 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Unfortunately it turns out that our CFLAG configure tests don't reliably
> > work with -Wold-style-definition. The problem is that the generated
> > program contains 'int main() {...}', which obviously is an old-style
> > definition. Which then causes a warning, which in turn causes the cflag
> > tests to fail because we run them with ac_c_werror_flag=yes.
>
> Ugh. I suspect main() might not be the only problem, either.
There's a few more, but they're far far less noisy. And I don't think
any change the results after the fix, because they don't check for
output on stderr (based on grepping through configure).
> > Upstream autoconf has fixed this in 2014 (1717921a), but since they've
> > not bothered to release since then...
>
> I wonder if there's any way to light a fire under them.
There's been talk about working towards a release a few months back:
https://lists.gnu.org/archive/html/autoconf/2020-03/msg00003.html
> > The easiest way that I can see to deal with that is to simply redefine
> > the relevant autoconf macro. For me that solves the vast majority of
> > these bleats in config.log. That's not particularly pretty, but we have
> > precedent for it... Since it's just 16 lines, I think we can live with
> > that?
>
> I don't really think that -Wold-style-definition is worth that.
Well, we don't need it for that, strictly speaking. Just using
AC_LANG_SOURCE is enough...
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 9+ messages in thread
end of thread, other threads:[~2020-06-09 06:13 UTC | newest]
Thread overview: 9+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-05-09 17:48 Add -Wold-style-definition to CFLAGS? Andres Freund <andres@anarazel.de>
2020-05-09 18:15 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-05-09 18:43 ` Andres Freund <andres@anarazel.de>
2020-05-09 23:11 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-05-10 01:37 ` Michael Paquier <michael@paquier.xyz>
2020-05-11 03:45 ` Alvaro Herrera <alvherre@2ndquadrant.com>
2020-06-09 05:47 ` Andres Freund <andres@anarazel.de>
2020-06-09 06:03 ` Tom Lane <tgl@sss.pgh.pa.us>
2020-06-09 06:13 ` Andres Freund <andres@anarazel.de>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox