Date: Tue, 17 Mar 2026 12:43:18 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Alexander Kuzmenkov <akuzmenkov@tigerdata.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>,
	Andres Freund <andres@anarazel.de>,
	Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>,
	Michael Paquier <michael@paquier.xyz>,
	Thomas Munro <thomas.munro@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Fix uninitialized xl_running_xacts padding
Message-ID: <ablMZvnGhX8h5GgK@ip-10-97-1-34.eu-west-3.compute.internal>
References: 
 <CAO6_XqpGYneOe357s4QhOTYpU6UEgk3JJrBiyeuk-9n=f7p=Vw@mail.gmail.com>
 <aoaj45foewpjtu6r5cs67yrx4en3pkurs23e4azv6tbikpw6c3@h3pnaqaksoeg>
 <b607e42e-34ff-414a-b727-dd5ec70babe3@timescale.com>
 <ed73b90b-e153-41ba-a626-2e13da42d6c8@iki.fi>
 <CALzhyqzKTRVsQGj+qDDRVs3Oo0EvffuQvVO0v4rbpWU=SoXKug@mail.gmail.com>
 <CALzhyqzLh9iciLMvT9T-Z+o1ekOPpeP2p2r7tYXNZ8JMSea3Zg@mail.gmail.com>
 <f2875d6b-5ff6-4744-b151-f5ab54cbd2c5@iki.fi>
 <CALzhyqy27==NkjnXYysF1zJj_K2rX5kncJZ9fEdTRVszTjJisg@mail.gmail.com>
 <70674869-2829-4b06-ab93-2f82ea51578c@iki.fi>
 <CALzhyqxrc1ZHYmf5V8NE+yMboqVg7xZrQM7K2c7VS0p1v8z42w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CALzhyqxrc1ZHYmf5V8NE+yMboqVg7xZrQM7K2c7VS0p1v8z42w@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ablMZvnGhX8h5GgK%40ip-10-97-1-34.eu-west-3.compute.internal>
Precedence: bulk

Hi,

On Mon, Mar 16, 2026 at 05:14:10PM +0100, Alexander Kuzmenkov wrote:
> I have removed the unnecessary memsets (for structs with no padding). With
> these changes, and removing the two WAL-related suppressions, the make
> installcheck under Valgrind passes.

Thanks for the patch!

Without the memset part of the patch, I got valgrind's things like:

    214 heap_multi_insert (heapam.c:2425)
    149 heap_inplace_update_and_unlock (heapam.c:6592)
      5 palloc (mcxt.c:1411)
      3 _bt_getroot (nbtpage.c:348)
      2 log_heap_prune_and_freeze (pruneheap.c:2171)
      2 LogCurrentRunningXacts (standby.c:1356)
      1 vacuumRedirectAndPlaceholder (spgvacuum.c:495)
      1 _bt_set_cleanup_info (nbtpage.c:234)
      1 ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.3)

and none with the patch applied. So the proposed changes look good to me.

One comment regarding the new memset(s) in the patch, I wonder if we should:

1/ Add a comment on top of them explaining why we are doing this and why
we don't use {0} (cf. Andres's point about C23 up-thread)

or

2/ Create a new macro, say INITIALIZE_PADDING or such with the comment on
top of it. That way, we could do things like:


+	INITIALIZE_PADDING(xlrec);
instead of
+	memset(&xlrec, 0, sizeof(xlrec));

I think that it would make the intent more clear and we could switch to {0} in a 
single place (if we feel the need) once C23 is required.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com