Re: Annoying warning in SerializeClientConnectionInfo

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)postgresql(dot)org, Andy Fan <zhihuifan1213(at)163(dot)com>
Subject: Re: Annoying warning in SerializeClientConnectionInfo
Date: 2025-08-12 02:34:32
Message-ID: aJqoODwa11zGnzsb@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2025-08-12 03:22:11 Excessive LOG messages from replication slot sync worker
Previous Message Chao Li 2025-08-12 02:09:42 Re: Patch 1 of GB18030-2022 support