public inbox for [email protected]
help / color / mirror / Atom feedRe: Fix uninitialized xl_running_xacts padding
2+ messages / 2 participants
[nested] [flat]
* Re: Fix uninitialized xl_running_xacts padding
@ 2026-02-16 00:17 Thomas Munro <[email protected]>
0 siblings, 1 reply; 2+ messages in thread
From: Thomas Munro @ 2026-02-16 00:17 UTC (permalink / raw)
To: Anthonin Bonnefoy <[email protected]>; +Cc: pgsql-hackers
On Fri, Feb 13, 2026 at 10:39 PM Anthonin Bonnefoy
<[email protected]> wrote:
> The 3 bytes of padding after subxid_overflow were left uninitialized,
> leading to the random 'ca ce 9b' data being written in the WAL. The
> attached patch fixes the issue by zeroing the xl_running_xacts
> structure in LogCurrentRunningXacts using MemSet.
Nitpick: the so-called universal zero initialiser syntax (var = {0})
is a nicer way to do this and generally preferred in new code AFAIK.
But in this case, it seems we don't actually worry about initialising
WAL padding bytes in general. valgrind.supp has an entry to prevent
warnings about it. Should we?
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: Fix uninitialized xl_running_xacts padding
@ 2026-02-16 01:10 Michael Paquier <[email protected]>
parent: Thomas Munro <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Michael Paquier @ 2026-02-16 01:10 UTC (permalink / raw)
To: Thomas Munro <[email protected]>; +Cc: Anthonin Bonnefoy <[email protected]>; pgsql-hackers
On Mon, Feb 16, 2026 at 01:17:56PM +1300, Thomas Munro wrote:
> Nitpick: the so-called universal zero initialiser syntax (var = {0})
> is a nicer way to do this and generally preferred in new code AFAIK.
My memory on the matter may be fuzzy, of course, but the initializer
does not guarantee that the padding bytes are initialized to zero
because the padding bytes are not associated to a member in the
structure. A memset(0), however, makes sure that the padding bytes
are full of zeros by taking into account the full size of the
structure. We could couple a {0} with some dummy fields in
xl_running_xacts, of course. But actually, there may be an even
smarter move in this case: LogCurrentRunningXacts() uses
MinSizeOfXactRunningXacts to store the data of a xl_running_xacts,
based on an offset of xl_running_xacts.xids. So we could move
subxid_overflow at the end of xl_running_xacts before xids, shaving
these padding bytes away while inserting the record's data.
> But in this case, it seems we don't actually worry about initialising
> WAL padding bytes in general. valgrind.supp has an entry to prevent
> warnings about it. Should we?
True about the initialization part, mostly I guess, still we tend to
worry about eliminating padding because these are wasted bytes in the
WAL records. For example, xlhp_freeze_plans has two bytes of padding,
that we eliminate while inserting its record by splitting the
FLEXIBLE_ARRAY_MEMBER part.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, 2-signature.asc)
download
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-02-16 01:10 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-02-16 00:17 Re: Fix uninitialized xl_running_xacts padding Thomas Munro <[email protected]>
2026-02-16 01:10 ` Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox