Thread: Re: further #include cleanup (IWYU)

Re: further #include cleanup (IWYU)

From
Alvaro Herrera
Date:
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)

From
Peter Eisentraut
Date:
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.