further #include cleanup (IWYU)
This is a continuation of [0]/messages/by-id/af837490-6b2f-46df-ba05-37ea6a6653fc@eisentraut.org ("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]: /messages/by-id/af837490-6b2f-46df-ba05-37ea6a6653fc@eisentraut.org
so a lot of the changes have similar patterns as [0]/messages/by-id/af837490-6b2f-46df-ba05-37ea6a6653fc@eisentraut.org.
[0]: /messages/by-id/af837490-6b2f-46df-ba05-37ea6a6653fc@eisentraut.org
/messages/by-id/af837490-6b2f-46df-ba05-37ea6a6653fc@eisentraut.org
Attachments:
0001-Remove-unused-include-s-from-backend-.c-files.patchtext/plain; charset=UTF-8; name=0001-Remove-unused-include-s-from-backend-.c-files.patchDownload+22-109
0002-Remove-unused-include-s-from-contrib-.c-files.patchtext/plain; charset=UTF-8; name=0002-Remove-unused-include-s-from-contrib-.c-files.patchDownload+33-122
0003-Remove-unused-include-s-from-pl-.c-files.patchtext/plain; charset=UTF-8; name=0003-Remove-unused-include-s-from-pl-.c-files.patchDownload+3-27
0004-Remove-unused-include-s-from-src-test-.c-files.patchtext/plain; charset=UTF-8; name=0004-Remove-unused-include-s-from-src-test-.c-files.patchDownload+4-34
0005-Remove-unused-include-s-from-bin-.c-files.patchtext/plain; charset=UTF-8; name=0005-Remove-unused-include-s-from-bin-.c-files.patchDownload+10-60
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/
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
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.
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" +#endifThis 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.
Seeing no further comments (or any easy alternatives), I have committed
this last patch as is.
Show quoted text
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" +#endifThis 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.
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)
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.