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
