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
