Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Enable -Wstrict-prototypes and -Wold-style-definition by default

  2. Use "foo(void)" for definitions of functions with no parameters.

  1. Use func(void) for functions with no parameters

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2025-12-03T14:51:25Z

    Hi hackers,
    
    In C standards till C17, func() means "unspecified parameters" while func(void)
    means "no parameters". The former disables compile time type checking and was
    marked obsolescent in C99 ([1]).
    
    This patch replaces empty parameter lists with explicit void to enable proper
    type checking and eliminate possible undefined behavior (see [1]) if the function
    is called with parameters. This also prevents real bugs (API misuse for example).
    
    Remarks:
    
    - C23 ([2]) made func() and func(void) equivalent and would produce an error
    if an argument is passed.
    
    - The patch has been generated with the help of a coccinelle script [3]. It does
    modify 8 functions and did not touch the few ones in .c "test" files (libpq_testclient.c
    for example).
    
    [1]: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
    [2]: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3220.pdf
    [3]: https://github.com/bdrouvot/coccinelle_on_pg/blob/main/misc/use_func_void.cocci
    
    Regards,
    
    -- 
    Bertrand Drouvot
    PostgreSQL Contributors Team
    RDS Open Source Databases
    Amazon Web Services: https://aws.amazon.com
    
  2. Re: Use func(void) for functions with no parameters

    Matthias van de Meent <boekewurm+postgres@gmail.com> — 2025-12-03T15:02:17Z

    On Wed, 3 Dec 2025 at 15:51, Bertrand Drouvot
    <bertranddrouvot.pg@gmail.com> wrote:
    >
    > Hi hackers,
    >
    > In C standards till C17, func() means "unspecified parameters" while func(void)
    > means "no parameters". The former disables compile time type checking and was
    > marked obsolescent in C99 ([1]).
    >
    > This patch replaces empty parameter lists with explicit void to enable proper
    > type checking and eliminate possible undefined behavior (see [1]) if the function
    > is called with parameters. This also prevents real bugs (API misuse for example).
    
    LGTM, thanks!
    
    I noticed the only changes here are for `static` definitions. Are we
    just more careful with normal functions, or does the compiler complain
    more easily about such "incomplete" definitions when they're in
    headers or need to be linked against?
    
    
    Kind regards,
    
    Matthias van de Meent
    Databricks (https://www.databricks.com)
    
    
    
    
  3. Re: Use func(void) for functions with no parameters

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-03T15:15:41Z

    Matthias van de Meent <boekewurm+postgres@gmail.com> writes:
    > I noticed the only changes here are for `static` definitions. Are we
    > just more careful with normal functions, or does the compiler complain
    > more easily about such "incomplete" definitions when they're in
    > headers or need to be linked against?
    
    Some years ago we had a buildfarm animal that would complain about
    this construct, so the tree used to be clean.  Probably it's just
    chance that these have only snuck into local functions.
    
    			regards, tom lane
    
    
    
    
  4. Re: Use func(void) for functions with no parameters

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2025-12-03T15:53:37Z

    Hi,
    
    On Wed, Dec 03, 2025 at 10:15:41AM -0500, Tom Lane wrote:
    > Matthias van de Meent <boekewurm+postgres@gmail.com> writes:
    > > I noticed the only changes here are for `static` definitions. Are we
    > > just more careful with normal functions, or does the compiler complain
    > > more easily about such "incomplete" definitions when they're in
    > > headers or need to be linked against?
    > 
    > Some years ago we had a buildfarm animal that would complain about
    > this construct, so the tree used to be clean.  Probably it's just
    > chance that these have only snuck into local functions.
    
    Thank you both for looking at it!
    
    The buildfarm animal remark makes me think to check with -Wstrict-prototypes
    and -Wold-style-definition. I just did that and found two more (added in v2
    attached) that the coccinelle script missed...
    
    Those new two (run_apply_worker() and usage()) are also static, so that's just
    chance.
    
    Regards,
    
    -- 
    Bertrand Drouvot
    PostgreSQL Contributors Team
    RDS Open Source Databases
    Amazon Web Services: https://aws.amazon.com
    
  5. Re: Use func(void) for functions with no parameters

    Nathan Bossart <nathandbossart@gmail.com> — 2025-12-03T16:10:10Z

    On Wed, Dec 03, 2025 at 03:53:37PM +0000, Bertrand Drouvot wrote:
    > The buildfarm animal remark makes me think to check with -Wstrict-prototypes
    > and -Wold-style-definition. I just did that and found two more (added in v2
    > attached) that the coccinelle script missed...
    > 
    > Those new two (run_apply_worker() and usage()) are also static, so that's just
    > chance.
    
    LGTM, too.  I'll take care of committing this shortly.
    
    -- 
    nathan
    
    
    
    
  6. Re: Use func(void) for functions with no parameters

    Nathan Bossart <nathandbossart@gmail.com> — 2025-12-03T16:56:42Z

    Committed.
    
    -- 
    nathan
    
    
    
    
  7. Re: Use func(void) for functions with no parameters

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-04T04:32:10Z

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
    > On Wed, Dec 03, 2025 at 10:15:41AM -0500, Tom Lane wrote:
    >> Some years ago we had a buildfarm animal that would complain about
    >> this construct, so the tree used to be clean.  Probably it's just
    >> chance that these have only snuck into local functions.
    
    > The buildfarm animal remark makes me think to check with -Wstrict-prototypes
    > and -Wold-style-definition. I just did that and found two more (added in v2
    > attached) that the coccinelle script missed...
    
    I looked into enabling -Wstrict-prototypes on one of my buildfarm
    animals, but the attempt failed because libreadline's headers are
    not clean.
    
    It looks like we could silence those warnings by #define'ing
    HAVE_STDARG_H and _FUNCTION_DEF before including the readline
    headers.  A quick test says that then the warnings do not appear,
    and psql's regression tests still pass.  But it would require
    a good deal more investigation of possible side-effects before
    I'd recommend actually doing that.
    
    			regards, tom lane
    
    
    
    
  8. Re: Use func(void) for functions with no parameters

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2025-12-04T10:04:37Z

    Hi,
    
    On Wed, Dec 03, 2025 at 11:32:10PM -0500, Tom Lane wrote:
    > Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
    > > The buildfarm animal remark makes me think to check with -Wstrict-prototypes
    > > and -Wold-style-definition. I just did that and found two more (added in v2
    > > attached) that the coccinelle script missed...
    > 
    > I looked into enabling -Wstrict-prototypes on one of my buildfarm
    > animals, but the attempt failed because libreadline's headers are
    > not clean.
    
    It took me some time to reproduce the errors...
    
    The reason is that gcc/clang treat certain directories (like /usr/include and
    /usr/local/include on my system) as "system headers" and suppress warnings from
    them, even with -Wsystem-headers.
    
    I finally reproduced the issue by installing readline in /opt/readline/include/:
    
    "
    In file included from /opt/readline/include/readline/readline.h:36,
                     from input.h:21,
                     from command.c:38:
    /opt/readline/include/readline/rltypedefs.h:35:1: error: function declaration isn’t a prototype [-Werror=strict-prototypes]
       35 | typedef int Function () __attribute__ ((deprecated));
          | ^~~~~~~
    /opt/readline/include/readline/rltypedefs.h:36:1: error: function declaration isn’t a prototype [-Werror=strict-prototypes]
       36 | typedef void VFunction () __attribute__ ((deprecated));
          | ^~~~~~~
    /opt/readline/include/readline/rltypedefs.h:37:1: error: function declaration isn’t a prototype [-Werror=strict-prototypes]
       37 | typedef char *CPFunction () __attribute__ ((deprecated));
          | ^~~~~~~
    /opt/readline/include/readline/rltypedefs.h:38:1: error: function declaration isn’t a prototype [-Werror=strict-prototypes]
       38 | typedef char **CPPFunction () __attribute__ ((deprecated));
          | ^~~~~~~
    /opt/readline/include/readline/readline.h:408:1: error: function declaration isn’t a prototype [-Werror=strict-prototypes]
      408 | extern int rl_message ();
          | ^~~~~~
    "
    
    This makes me wonder: there might be other headers with similar issues that 
    we just don't see because they're in "system" directories.
    
    Out of curiosity, where is readline installed on your buildfarm animal?
    
    > It looks like we could silence those warnings by #define'ing
    > HAVE_STDARG_H and _FUNCTION_DEF before including the readline
    > headers.  A quick test says that then the warnings do not appear,
    > and psql's regression tests still pass.  But it would require
    > a good deal more investigation of possible side-effects before
    > I'd recommend actually doing that.
    
    Yeah, what about using Pragma Directives instead, like in the attached? I don't
    see any errors with those and that looks safer to use as it only suppresses
    compiler warnings.
    
    Remark: I've read the comment about "pragma GCC diagnostic" in c.h but I think
    it's safe to use for -Wstrict-prototypes (as old enough).
    
    Regards,
    
    -- 
    Bertrand Drouvot
    PostgreSQL Contributors Team
    RDS Open Source Databases
    Amazon Web Services: https://aws.amazon.com
    
  9. Re: Use func(void) for functions with no parameters

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-12-04T15:17:28Z

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
    > On Wed, Dec 03, 2025 at 11:32:10PM -0500, Tom Lane wrote:
    >> I looked into enabling -Wstrict-prototypes on one of my buildfarm
    >> animals, but the attempt failed because libreadline's headers are
    >> not clean.
    
    > It took me some time to reproduce the errors...
    
    > The reason is that gcc/clang treat certain directories (like /usr/include and
    > /usr/local/include on my system) as "system headers" and suppress warnings from
    > them, even with -Wsystem-headers.
    
    Oh, duh.  You guessed correctly: I was trying this on BF animal
    indri, which gets a lot of stuff from MacPorts and therefore these
    headers are under /opt/local/include.  I wonder whether I should
    adjust its build flags to treat that as a system directory.
    It hasn't been a problem up to now, but ...
    
    > Yeah, what about using Pragma Directives instead, like in the
    > attached?
    
    Yeah, a pragma is probably safer than what I was thinking about.
    But I'd be inclined to just use "#pragma GCC system_header" in
    input.h, since that's already tested and used elsewhere in the tree.
    There's little enough other stuff in that file that I think we
    could just do it, and not bother breaking out a sub-include file
    like we did in plperl and plpython.
    
    In any case, I don't think we should bother unless there's a push to
    enable -Wstrict-prototypes by default, which I've not heard being
    proposed.
    
    			regards, tom lane
    
    
    
    
  10. Re: Use func(void) for functions with no parameters

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2025-12-05T06:35:59Z

    Hi,
    
    On Thu, Dec 04, 2025 at 10:17:28AM -0500, Tom Lane wrote:
    > Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
    > > Yeah, what about using Pragma Directives instead, like in the
    > > attached?
    > 
    > Yeah, a pragma is probably safer than what I was thinking about.
    > But I'd be inclined to just use "#pragma GCC system_header" in
    > input.h, since that's already tested and used elsewhere in the tree.
    
    Agree.
    
    > In any case, I don't think we should bother unless there's a push to
    > enable -Wstrict-prototypes by default, which I've not heard being
    > proposed.
    
    Now that the code tree is clean, I think that this is a good timing for it.
    
    We have been tracking and cleaning those in commits cdf4b9aff2, 0e72b9d440,
    7069dbcc31, f1283ed6cc, 7b66e2c086, e95126cf04, 9f7c527af3 and recently in
    9b05e2ec08a, so I think that it would make sense to "ensure" it's always tracked.
    
    This is not just about coding style but also prevents undefined behavior (see
    [1], §6.5.2.2/6). And C23 made foo() and foo(void) equivalent (see [2],
    §6.7.7.4/13), so this would align with it.
    
    If that sounds reasonable, I'd happy to work on it, thoughts?
    
    [1]: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf
    [2]: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3220.pdf
    
    Regards,
    
    -- 
    Bertrand Drouvot
    PostgreSQL Contributors Team
    RDS Open Source Databases
    Amazon Web Services: https://aws.amazon.com