Add -Wold-style-definition to CFLAGS?
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t42351psql -h localhost -U postgresBuilt from patchset v1 (message #1), July 28, 2026 at 02:40 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t42351_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t42351_1 && git checkout t42351_1Patchset v1 (message #1) is on t42351_1
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
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
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
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
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
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
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
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
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