Re: Annoying warning in SerializeClientConnectionInfo

From: Jacob Champion <jacob(dot)champion(at)enterprisedb(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
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 20:44:32
Message-ID: CAOYmi+=UjAHzm0MXBvRCYf0Fz0OzgXh0K4wXM5aBdkV7Ei1T_w@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Aug 11, 2025 at 7:34 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> And the rest was looking OK, so appending a
> PG_USED_FOR_ASSERTS_ONLY in the declaration seems OK from here.

If we're the first to use the attribute this way, I think I'd prefer
to put it on the definition only.

> I'd rather keep the sanity check on maxsize, even if it means to have
> a tweak based on the size of SerializedClientConnectionInfo.

I don't think I understand what you mean by this? I don't want to get
rid of the check, but I was wondering if we could strengthen the
behavior on HEAD to raise an ERROR regardless of whether assertions
are enabled or not. Similar to the approach taken by
SerializeComboCIDState().

I think the PG_USED_FOR_ASSERTS_ONLY fix is preferable for backport,
so I don't want to get in the way of that approach.

> 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.

Agreed.

Thanks,
--Jacob

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jacob Champion 2025-08-12 20:46:28 Re: Annoying warning in SerializeClientConnectionInfo
Previous Message Tom Lane 2025-08-12 20:19:02 Re: `pg_ctl init` crashes when run concurrently; semget(2) suspected