Обсуждение: Use pg_icu_unicode_version(void) instead of pg_icu_unicode_version()
Hi hackers, Standard practice in PostgreSQL is to use "foo(void)" instead of "foo()", as the latter looks like an "old-style" function declaration. 9b05e2ec08a did fix all the ones reported by -Wstrict-prototypes. af2d4ca191a4 introduced a new one, this patch fixes it. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Вложения
> On Feb 27, 2026, at 12:46, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote: > > Hi hackers, > > Standard practice in PostgreSQL is to use "foo(void)" instead of "foo()", as the > latter looks like an "old-style" function declaration. 9b05e2ec08a did fix > all the ones reported by -Wstrict-prototypes. > > af2d4ca191a4 introduced a new one, this patch fixes it. > > Regards, > > -- > Bertrand Drouvot > PostgreSQL Contributors Team > RDS Open Source Databases > Amazon Web Services: https://aws.amazon.com > <v1-0001-Use-pg_icu_unicode_version-void-instead-of-pg_icu.patch> This patch is straightforward. What I'm interested in is the broader policy: when reviewing patches, if we encounter a foo() declaration, should we consistentlyrequest a change to foo(void)? If yes, the standard should be documented somewhere. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Hi, On Fri, Feb 27, 2026 at 02:04:30PM +0800, Chao Li wrote: > > > What I'm interested in is the broader policy: when reviewing patches, > if we encounter a foo() declaration, should we consistently request a change to foo(void)? > If yes, the standard should be documented somewhere. I think that they should be consistently fixed for the reasons mentioned in [1], and that the best way to achieve this goal would be to enable -Wstrict-prototypes by default ([2]). [1]: https://postgr.es/m/aTBObQPg%2Bps5I7vl%40ip-10-97-1-34.eu-west-3.compute.internal [2]: https://postgr.es/m/aTJ9T8HyJN3D024w%40ip-10-97-1-34.eu-west-3.compute.internal Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
On 2/27/26 01:04, Chao Li wrote: > What I'm interested in is the broader policy: when reviewing > patches, if we encounter a foo() declaration, should we consistently > request a change to foo(void)? If yes, the standard should be > documented somewhere. Perhaps here? https://wiki.postgresql.org/wiki/Committing_checklist -- Joe Conway PostgreSQL Contributors Team Amazon Web Services: https://aws.amazon.com
On 27.02.26 07:45, Bertrand Drouvot wrote: > Hi, > > On Fri, Feb 27, 2026 at 02:04:30PM +0800, Chao Li wrote: >> >> >> What I'm interested in is the broader policy: when reviewing patches, >> if we encounter a foo() declaration, should we consistently request a change to foo(void)? >> If yes, the standard should be documented somewhere. > > I think that they should be consistently fixed for the reasons mentioned in > [1], and that the best way to achieve this goal would be to enable -Wstrict-prototypes > by default ([2]). Yes, why not add -Wstrict-prototypes and perhaps -Wold-style-definition to the standard warnings. Then we don't have to keep chasing these manually.
On 2/27/26 3:58 PM, Joe Conway wrote: > On 2/27/26 01:04, Chao Li wrote: >> What I'm interested in is the broader policy: when reviewing >> patches, if we encounter a foo() declaration, should we consistently >> request a change to foo(void)? If yes, the standard should be >> documented somewhere. > > Perhaps here? > > https://wiki.postgresql.org/wiki/Committing_checklist Nah, I think we should just enable the warnings unless we have a good reason not to. Andreas
On Fri, 2026-02-27 at 04:46 +0000, Bertrand Drouvot wrote:
> Hi hackers,
>
> Standard practice in PostgreSQL is to use "foo(void)" instead of
> "foo()", as the
> latter looks like an "old-style" function declaration. 9b05e2ec08a
> did fix
> all the ones reported by -Wstrict-prototypes.
>
> af2d4ca191a4 introduced a new one, this patch fixes it.
Committed, thank you.
Regards,
Jeff Davis
Hi, On Mon, Mar 02, 2026 at 07:36:20PM +0100, Peter Eisentraut wrote: > On 27.02.26 07:45, Bertrand Drouvot wrote: > > Hi, > > > > On Fri, Feb 27, 2026 at 02:04:30PM +0800, Chao Li wrote: > > > > > > > > > What I'm interested in is the broader policy: when reviewing patches, > > > if we encounter a foo() declaration, should we consistently request a change to foo(void)? > > > If yes, the standard should be documented somewhere. > > > > I think that they should be consistently fixed for the reasons mentioned in > > [1], and that the best way to achieve this goal would be to enable -Wstrict-prototypes > > by default ([2]). > > Yes, why not add -Wstrict-prototypes and perhaps -Wold-style-definition to > the standard warnings. Then we don't have to keep chasing these manually. Yeah, I'll look at adding those. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
On 09.03.26 08:57, Bertrand Drouvot wrote: > Hi, > > On Mon, Mar 02, 2026 at 07:36:20PM +0100, Peter Eisentraut wrote: >> On 27.02.26 07:45, Bertrand Drouvot wrote: >>> Hi, >>> >>> On Fri, Feb 27, 2026 at 02:04:30PM +0800, Chao Li wrote: >>>> >>>> >>>> What I'm interested in is the broader policy: when reviewing patches, >>>> if we encounter a foo() declaration, should we consistently request a change to foo(void)? >>>> If yes, the standard should be documented somewhere. >>> >>> I think that they should be consistently fixed for the reasons mentioned in >>> [1], and that the best way to achieve this goal would be to enable -Wstrict-prototypes >>> by default ([2]). >> >> Yes, why not add -Wstrict-prototypes and perhaps -Wold-style-definition to >> the standard warnings. Then we don't have to keep chasing these manually. > > Yeah, I'll look at adding those. I played with this a little bit. A problem I found is that the generated configure code itself generates its test programs with 'main()', and so with these warnings, many of these tests will fail. So you'd need to create some different arrangement where you test for the warnings but only add the flags at the end of configure. But also, my research indicates that -Wstrict-prototypes and -Wold-style-definition are available in all supported gcc and clang versions, so maybe you could avoid this problem by not testing for them and just unconditionally adding them at the end.
Hi, On Mon, Mar 09, 2026 at 09:44:45AM +0100, Peter Eisentraut wrote: > On 09.03.26 08:57, Bertrand Drouvot wrote: > > Hi, > > > > On Mon, Mar 02, 2026 at 07:36:20PM +0100, Peter Eisentraut wrote: > > > On 27.02.26 07:45, Bertrand Drouvot wrote: > > > > Hi, > > > > > > > > On Fri, Feb 27, 2026 at 02:04:30PM +0800, Chao Li wrote: > > > > > > > > > > > > > > > What I'm interested in is the broader policy: when reviewing patches, > > > > > if we encounter a foo() declaration, should we consistently request a change to foo(void)? > > > > > If yes, the standard should be documented somewhere. > > > > > > > > I think that they should be consistently fixed for the reasons mentioned in > > > > [1], and that the best way to achieve this goal would be to enable -Wstrict-prototypes > > > > by default ([2]). > > > > > > Yes, why not add -Wstrict-prototypes and perhaps -Wold-style-definition to > > > the standard warnings. Then we don't have to keep chasing these manually. > > > > Yeah, I'll look at adding those. > > I played with this a little bit. A problem I found is that the generated > configure code itself generates its test programs with 'main()', and so with > these warnings, many of these tests will fail. So you'd need to create some > different arrangement where you test for the warnings but only add the flags > at the end of configure. > > But also, my research indicates that -Wstrict-prototypes and > -Wold-style-definition are available in all supported gcc and clang > versions, so maybe you could avoid this problem by not testing for them and > just unconditionally adding them at the end. Yeah, I did observe the same. I moved the new flags late enough in configure.ac to avoid any error (for example, PGAC_PRINTF_ARCHETYPE which uses -Werror and would fail to detect gnu_printf if -Wstrict-prototypes is active) leading to the mingw_cross_warning CI task failing. I just shared the patch in a dedicated thread [1]. [1]: https://postgr.es/m/aa73q1aT0A3/vke/@ip-10-97-1-34.eu-west-3.compute.internal Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com