Thread
Commits
-
Remove unused #include's from bin .c files
- ecb5af779877 18.0 landed
-
Remove unused #include's from contrib, pl, test .c files
- 9be4e5d293b5 18.0 landed
-
Remove unused #include's from backend .c files
- e18512c000ee 18.0 landed
-
Replace most usages of ntoh[ls] and hton[sl] with pg_bswap.h.
- 0ba99c84e8c7 11.0 cited
-
further #include cleanup (IWYU)
Peter Eisentraut <peter@eisentraut.org> — 2024-10-20T07:50:00Z
This is a continuation of [0] ("backend *.c #include cleanup (IWYU)"), which removed a bunch of unneeded #include's, found by the include-what-you-use (IWYU) tool. I went through the rest of the code and did similar cleanups. The patches are organized by code area, but they are otherwise not separate. The first patch ("Remove unused #include's from backend .c files") is mostly stuff that is new since [0]. The remaining patches deal with code that wasn't touched by [0], so a lot of the changes have similar patterns as [0]. [0]: https://www.postgresql.org/message-id/flat/af837490-6b2f-46df-ba05-37ea6a6653fc%40eisentraut.org -
Re: further #include cleanup (IWYU)
Alvaro Herrera <alvherre@alvh.no-ip.org> — 2024-10-20T09:37:36Z
On 2024-Oct-20, Peter Eisentraut wrote: > diff --git a/contrib/tablefunc/tablefunc.h b/contrib/tablefunc/tablefunc.h > index 2009382ce7d..b78030044b5 100644 > --- a/contrib/tablefunc/tablefunc.h > +++ b/contrib/tablefunc/tablefunc.h > @@ -34,6 +34,4 @@ > #ifndef TABLEFUNC_H > #define TABLEFUNC_H > > -#include "fmgr.h" > - > #endif /* TABLEFUNC_H */ You could as well just delete this file. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
-
Re: further #include cleanup (IWYU)
Alvaro Herrera <alvherre@alvh.no-ip.org> — 2024-10-20T09:53:13Z
On 2024-Oct-20, Peter Eisentraut wrote: > diff --git a/src/bin/pg_dump/pg_backup_utils.c b/src/bin/pg_dump/pg_backup_utils.c > index a0045cf5e58..80715979a1a 100644 > --- a/src/bin/pg_dump/pg_backup_utils.c > +++ b/src/bin/pg_dump/pg_backup_utils.c > @@ -13,7 +13,9 @@ > */ > #include "postgres_fe.h" > > +#ifdef WIN32 > #include "parallel.h" > +#endif > #include "pg_backup_utils.h" This seems quite weird and I think it's just because exit_nicely() wants to do _endthreadex(). Maybe it'd be nicer to add a WIN32-specific on_exit_nicely_list() callback that does that in parallel.c, and do away with the inclusion of parallel.h in pg_backup_utils.c entirely? > diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c > index a09247fae47..78e91f6e2dc 100644 > --- a/src/bin/pg_dump/parallel.c > +++ b/src/bin/pg_dump/parallel.c > @@ -63,7 +63,9 @@ > #include "fe_utils/string_utils.h" > #include "parallel.h" > #include "pg_backup_utils.h" > +#ifdef WIN32 > #include "port/pg_bswap.h" > +#endif This looks really strange, but then parallel.c seems to have embedded its own portability layer within itself. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "I'm impressed how quickly you are fixing this obscure issue. I came from MS SQL and it would be hard for me to put into words how much of a better job you all are doing on [PostgreSQL]." Steve Midgley, http://archives.postgresql.org/pgsql-sql/2008-08/msg00000.php
-
Re: further #include cleanup (IWYU)
Peter Eisentraut <peter@eisentraut.org> — 2024-10-28T09:41:17Z
On 20.10.24 11:37, Alvaro Herrera wrote: > On 2024-Oct-20, Peter Eisentraut wrote: > >> diff --git a/contrib/tablefunc/tablefunc.h b/contrib/tablefunc/tablefunc.h >> index 2009382ce7d..b78030044b5 100644 >> --- a/contrib/tablefunc/tablefunc.h >> +++ b/contrib/tablefunc/tablefunc.h >> @@ -34,6 +34,4 @@ >> #ifndef TABLEFUNC_H >> #define TABLEFUNC_H >> >> -#include "fmgr.h" >> - >> #endif /* TABLEFUNC_H */ > > You could as well just delete this file. I have committed it with the file deleted.
-
Re: further #include cleanup (IWYU)
Peter Eisentraut <peter@eisentraut.org> — 2024-10-28T09:45:21Z
On 20.10.24 11:53, Alvaro Herrera wrote: > On 2024-Oct-20, Peter Eisentraut wrote: >> diff --git a/src/bin/pg_dump/pg_backup_utils.c b/src/bin/pg_dump/pg_backup_utils.c >> index a0045cf5e58..80715979a1a 100644 >> --- a/src/bin/pg_dump/pg_backup_utils.c >> +++ b/src/bin/pg_dump/pg_backup_utils.c >> @@ -13,7 +13,9 @@ >> */ >> #include "postgres_fe.h" >> >> +#ifdef WIN32 >> #include "parallel.h" >> +#endif >> #include "pg_backup_utils.h" > > This seems quite weird and I think it's just because exit_nicely() wants > to do _endthreadex(). Maybe it'd be nicer to add a WIN32-specific > on_exit_nicely_list() callback that does that in parallel.c, and do away > with the inclusion of parallel.h in pg_backup_utils.c entirely? I was thinking the same thing. But maybe that should be a separate project. >> diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c >> index a09247fae47..78e91f6e2dc 100644 >> --- a/src/bin/pg_dump/parallel.c >> +++ b/src/bin/pg_dump/parallel.c >> @@ -63,7 +63,9 @@ >> #include "fe_utils/string_utils.h" >> #include "parallel.h" >> #include "pg_backup_utils.h" >> +#ifdef WIN32 >> #include "port/pg_bswap.h" >> +#endif > > This looks really strange, but then parallel.c seems to have embedded > its own portability layer within itself. The reason for this one is that pgpipe() uses pg_hton16() and pg_hton32(). We could use htons() and htonl() here instead. That would effectively revert that part of commit 0ba99c84e8c.
-
Re: further #include cleanup (IWYU)
Peter Eisentraut <peter@eisentraut.org> — 2024-11-06T10:50:34Z
Seeing no further comments (or any easy alternatives), I have committed this last patch as is. On 28.10.24 10:45, Peter Eisentraut wrote: > On 20.10.24 11:53, Alvaro Herrera wrote: >> On 2024-Oct-20, Peter Eisentraut wrote: >>> diff --git a/src/bin/pg_dump/pg_backup_utils.c b/src/bin/pg_dump/ >>> pg_backup_utils.c >>> index a0045cf5e58..80715979a1a 100644 >>> --- a/src/bin/pg_dump/pg_backup_utils.c >>> +++ b/src/bin/pg_dump/pg_backup_utils.c >>> @@ -13,7 +13,9 @@ >>> */ >>> #include "postgres_fe.h" >>> +#ifdef WIN32 >>> #include "parallel.h" >>> +#endif >>> #include "pg_backup_utils.h" >> >> This seems quite weird and I think it's just because exit_nicely() wants >> to do _endthreadex(). Maybe it'd be nicer to add a WIN32-specific >> on_exit_nicely_list() callback that does that in parallel.c, and do away >> with the inclusion of parallel.h in pg_backup_utils.c entirely? > > I was thinking the same thing. But maybe that should be a separate > project. > >>> diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c >>> index a09247fae47..78e91f6e2dc 100644 >>> --- a/src/bin/pg_dump/parallel.c >>> +++ b/src/bin/pg_dump/parallel.c >>> @@ -63,7 +63,9 @@ >>> #include "fe_utils/string_utils.h" >>> #include "parallel.h" >>> #include "pg_backup_utils.h" >>> +#ifdef WIN32 >>> #include "port/pg_bswap.h" >>> +#endif >> >> This looks really strange, but then parallel.c seems to have embedded >> its own portability layer within itself. > > The reason for this one is that pgpipe() uses pg_hton16() and > pg_hton32(). We could use htons() and htonl() here instead. That would > effectively revert that part of commit 0ba99c84e8c. > >
-
Re: further #include cleanup (IWYU)
Matthias van de Meent <boekewurm+postgres@gmail.com> — 2025-01-27T14:10:18Z
On Wed, 6 Nov 2024 at 11:50, Peter Eisentraut <peter@eisentraut.org> wrote: > > Seeing no further comments (or any easy alternatives), I have committed > this last patch as is. I just noticed that the #define for MaxArraySize in utils/array.h uses MaxAllocSize without including the utils/memutils.h header. Is that on purpose and is the user expected to include both headers, or should utils/memutils.h be included in utils/array.h? Kind regards, Matthias van de Meent Neon (https://neon.tech)
-
Re: further #include cleanup (IWYU)
Peter Eisentraut <peter@eisentraut.org> — 2025-01-29T11:35:00Z
On 27.01.25 15:10, Matthias van de Meent wrote: > On Wed, 6 Nov 2024 at 11:50, Peter Eisentraut <peter@eisentraut.org> wrote: >> >> Seeing no further comments (or any easy alternatives), I have committed >> this last patch as is. > > I just noticed that the #define for MaxArraySize in utils/array.h uses > MaxAllocSize without including the utils/memutils.h header. Is that on > purpose and is the user expected to include both headers, or should > utils/memutils.h be included in utils/array.h? I have found a number of cases like this, where a macro definition in a header uses a symbol that is defined in another header that is not included. This is not something that the IWYU tool set can track. In general, this kind of thing is probably best avoided, but it would probably require individual investigation of each case.