On 2017-06-23 19:21:57 +0100, Simon Riggs wrote: > On 23 June 2017 at 08:23, Simon Riggs <si...@2ndquadrant.com> wrote: > > On 23 June 2017 at 08:21, Andres Freund <and...@anarazel.de> wrote: > >> On 2017-06-07 10:17:31 -0700, Andres Freund wrote: > >>> On 2017-05-08 09:12:13 -0400, Tom Lane wrote: > >>> > Simon Riggs <si...@2ndquadrant.com> writes: > >>> > > So rearranged code a little to keep it lean. > >>> > > >>> > Didn't you break it with that? As it now stands, the memcpy will > >>> > copy the nonzero value. > >>> > >>> I've not seen a fix and/or alleviating comment about this so far. Did I > >>> miss something? > >> > >> Simon, FWIW, I plan to either revert or fix this up soon-ish. Unless > >> you're going to actually respond on this thread? > > > > Sorry, I've only just seen Tom's reply. Will fix. > > I don't see a bug. Perhaps I'm tired and can't see it yet. > > Will fix if you thwack me with the explanation.
Wasn't my complaint, but here we go: Previous code: /* * Ignore the SubXID array if it has overflowed, unless the snapshot was * taken during recovey - in that case, top-level XIDs are in subxip as * well, and we mustn't lose them. */ if (serialized_snapshot.suboverflowed && !snapshot->takenDuringRecovery) serialized_snapshot.subxcnt = 0; /* Copy struct to possibly-unaligned buffer */ memcpy(start_address, &serialized_snapshot, sizeof(SerializedSnapshotData)); i.e. if suboverflowed, start_address would contain subxcnt = 0. New code: /* Copy struct to possibly-unaligned buffer */ memcpy(start_address, &serialized_snapshot, sizeof(SerializedSnapshotData)); /* Copy XID array */ if (snapshot->xcnt > 0) memcpy((TransactionId *) (start_address + sizeof(SerializedSnapshotData)), snapshot->xip, snapshot->xcnt * sizeof(TransactionId)); /* * Copy SubXID array. Don't bother to copy it if it had overflowed, * though, because it's not used anywhere in that case. Except if it's a * snapshot taken during recovery; all the top-level XIDs are in subxip as * well in that case, so we mustn't lose them. */ if (serialized_snapshot.suboverflowed && !snapshot->takenDuringRecovery) serialized_snapshot.subxcnt = 0; Here the copy is done before subxcnt = 0. - Andres -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers