Re: Annoying warning in SerializeClientConnectionInfo - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Annoying warning in SerializeClientConnectionInfo
Date
Msg-id aJqoODwa11zGnzsb@paquier.xyz
Whole thread Raw
In response to Re: Annoying warning in SerializeClientConnectionInfo  (Jacob Champion <jacob.champion@enterprisedb.com>)
Responses Re: Annoying warning in SerializeClientConnectionInfo
List pgsql-hackers
On Mon, Aug 11, 2025 at 04:30:30PM -0700, Jacob Champion wrote:
> Probably
>     https://github.com/gcc-mirror/gcc/commit/0eac9cfee
> which was committed last month.
>
> Andy Fan reported this as well [1] but I did not see it at the time.
> :( Sorry, Andy, my email had changed.

Warning remains undetected here.

>> The most obvious fix is to slap on a PG_USED_FOR_ASSERTS_ONLY. However, we so
>> far don't seem to have used it for function parameters... But I don't see a
>> problem with starting to do so.

I'm guessing that it should be OK, pg_attribute_unused is only
available for __GNUC__, still wondered if MinGW+gcc would like that.

[ .. 30 minutes later, after a test .. ]

I have kicked one job, to note that the libpq build is broken on
Windows, bit due to a different thing because a bunch of other CF bot
jobs are failing the same way:
https://github.com/michaelpq/postgres/runs/47868772065

And the rest was looking OK, so appending a
PG_USED_FOR_ASSERTS_ONLY in the declaration seems OK from here.

> WFM. Do you have any opinions on our use of maxsize in general? I
> think there are other serialization functions that just assert, but it
> looks like some are more actively throwing errors if there's not
> enough space. Given the subject matter here I'm wondering if we should
> take the stricter approach.

I'd rather keep the sanity check on maxsize, even if it means to have
a tweak based on the size of SerializedClientConnectionInfo.  If you
feel differently about that as the original author of this code, I'd
be OK with what you want to prioritize here.

Another thing that we can do is use an USE_ASSERT_CHECKING around the
variable getting set, but as far as I can see the
PG_USED_FOR_ASSERTS_ONLY in the function declaration should work fine.
If the buildfarm blurps on the first approach, we could always use the
second approach as fallback.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Chao Li
Date:
Subject: Re: Patch 1 of GB18030-2022 support
Next
From: Fujii Masao
Date:
Subject: Excessive LOG messages from replication slot sync worker