Re: pgsql: Store 2PC GID in commit/abort WAL recs for logical decoding

On 28/03/18 19:47, Simon Riggs wrote:
> Store 2PC GID in commit/abort WAL recs for logical decoding

This forgot to update the comments in xl_xact_commit and xl_xact_abort, 
for the new fields.

> +
> +               if (parsed->xinfo & XACT_XINFO_HAS_GID)
> +               {
> +                       int gidlen;
> +                       strcpy(parsed->twophase_gid, data);
> +                       gidlen = strlen(parsed->twophase_gid) + 1;
> +                       data += MAXALIGN(gidlen);
> +               }
> +       }
> +
> +       if (parsed->xinfo & XACT_XINFO_HAS_ORIGIN)
> +       {
> +               xl_xact_origin xl_origin;
> +
> +               /* we're only guaranteed 4 byte alignment, so copy onto stack */
> +               memcpy(&xl_origin, data, sizeof(xl_origin));
> +
> +               parsed->origin_lsn = xl_origin.origin_lsn;
> +               parsed->origin_timestamp = xl_origin.origin_timestamp;
> +
> +               data += sizeof(xl_xact_origin);
>         }

There seems to be some confusion on the padding here. Firstly, what's 
the point of zero-padding the GID length to the next MAXALIGN boundary, 
which would be 8 bytes on 64-bit systems, if the start is only 
guaranteed 4-byte alignment, per the comment at the memcpy() above. 
Secondly, if we're memcpying the fields that follow anyway, why bother 
with any alignment padding at all?

I propose the attached.

- Heikki