Thread: Re: pg_checksums: Reorder headers in alphabetical order
On Fri, Sep 20, 2024 at 07:20:15PM +0200, Michael Banck wrote: > I noticed two headers are not in alphabetical order in pg_checkums.c, > patch attached. This appears to be commit 280e5f1's fault. Will fix. -- nathan
On Fri, Sep 20, 2024 at 01:56:16PM -0500, Nathan Bossart wrote: > On Fri, Sep 20, 2024 at 07:20:15PM +0200, Michael Banck wrote: >> I noticed two headers are not in alphabetical order in pg_checkums.c, >> patch attached. > > This appears to be commit 280e5f1's fault. Will fix. Committed, thanks! -- nathan
On Fri, Sep 20, 2024 at 03:20:16PM -0500, Nathan Bossart wrote: > On Fri, Sep 20, 2024 at 01:56:16PM -0500, Nathan Bossart wrote: > > On Fri, Sep 20, 2024 at 07:20:15PM +0200, Michael Banck wrote: > >> I noticed two headers are not in alphabetical order in pg_checkums.c, > >> patch attached. > > > > This appears to be commit 280e5f1's fault. Will fix. Oops, that was my fault then :) > Committed, thanks! Thanks! Michael
On 2024/09/21 5:20, Nathan Bossart wrote: > On Fri, Sep 20, 2024 at 01:56:16PM -0500, Nathan Bossart wrote: >> On Fri, Sep 20, 2024 at 07:20:15PM +0200, Michael Banck wrote: >>> I noticed two headers are not in alphabetical order in pg_checkums.c, >>> patch attached. >> >> This appears to be commit 280e5f1's fault. Will fix. > > Committed, thanks! I don’t have any objections to this commit, but I’d like to confirm whether we really want to proactively reorder #include directives, even for standard C library headers. I’m asking because I know there are several source files, like xlog.c and syslogger.c, where such #include directives aren't in alphabetical order. I understand we usually reorder #include directives for PostgreSQL header files, though. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Fujii Masao <masao.fujii@oss.nttdata.com> writes:
> I don’t have any objections to this commit, but I’d like to confirm
> whether we really want to proactively reorder #include directives,
> even for standard C library headers.
I'm hesitant to do that. We can afford to insist that our own header
files be inclusion-order-independent, because we have the ability to
fix any problems that might arise. We have no ability to do something
about it if the system headers on $random_platform have inclusion
order dependencies. (In fact, I'm fairly sure there are already
places in plperl and plpython where we know we have to be careful
about inclusion order around those languages' headers.)
So I would tread pretty carefully around making changes of this
type, especially in long-established code. I have no reason to
think that the committed patch will cause any problems, but
I do think it's mostly asking for trouble.
regards, tom lane
On 2024/09/21 12:09, Tom Lane wrote: > Fujii Masao <masao.fujii@oss.nttdata.com> writes: >> I don’t have any objections to this commit, but I’d like to confirm >> whether we really want to proactively reorder #include directives, >> even for standard C library headers. > > I'm hesitant to do that. We can afford to insist that our own header > files be inclusion-order-independent, because we have the ability to > fix any problems that might arise. We have no ability to do something > about it if the system headers on $random_platform have inclusion > order dependencies. (In fact, I'm fairly sure there are already > places in plperl and plpython where we know we have to be careful > about inclusion order around those languages' headers.) > > So I would tread pretty carefully around making changes of this > type, especially in long-established code. I have no reason to > think that the committed patch will cause any problems, but > I do think it's mostly asking for trouble. Sounds reasonable to me. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
On Sat, Sep 21, 2024 at 02:48:32PM +0900, Fujii Masao wrote: > On 2024/09/21 12:09, Tom Lane wrote: >> Fujii Masao <masao.fujii@oss.nttdata.com> writes: >> > I don´t have any objections to this commit, but I´d like to confirm >> > whether we really want to proactively reorder #include directives, >> > even for standard C library headers. >> >> I'm hesitant to do that. We can afford to insist that our own header >> files be inclusion-order-independent, because we have the ability to >> fix any problems that might arise. We have no ability to do something >> about it if the system headers on $random_platform have inclusion >> order dependencies. (In fact, I'm fairly sure there are already >> places in plperl and plpython where we know we have to be careful >> about inclusion order around those languages' headers.) >> >> So I would tread pretty carefully around making changes of this >> type, especially in long-established code. I have no reason to >> think that the committed patch will cause any problems, but >> I do think it's mostly asking for trouble. > > Sounds reasonable to me. Oh, sorry. I thought it was project policy to keep these alphabetized, and I was unaware of the past problems with system header ordering. I'll keep this in mind in the future. -- nathan