From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | pgsql-committers(at)postgresql(dot)org |
Subject: | pgsql: Fix multiple portability issues in pg_upgrade's rewriteVisibilit |
Date: | 2016-10-01 00:41:09 |
Message-ID: | E1bq8MX-0001Lj-VX@gemulon.postgresql.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers |
Fix multiple portability issues in pg_upgrade's rewriteVisibilityMap().
This is new code in 9.6, and evidently we missed out testing it as
thoroughly as it should have been. Bugs fixed here:
1. Use binary not text mode to open the files on Windows. Before, if
the visibility map chanced to contain two bytes that looked like \r\n,
Windows' read() would convert that to \n, which both corrupts the map
data and causes the file to look shorter than it should. Unless you
were *very* unlucky and had an exact multiple of 8K such occurrences
in each VM file, this would cause pg_upgrade to report a failure,
though with a rather obscure error message.
2. The code for copying rebuilt bytes into the output was simply wrong.
It chanced to work okay on little-endian machines but would emit the
bytes in the wrong order on big-endian, leading to silent corruption
of the visibility map data.
3. The code was careless about alignment of the working buffers. Given
all three of an alignment-picky architecture, a compiler that chooses
to put the new_vmbuf[] local variable at an odd starting address, and
a checksum-enabled database, pg_upgrade would dump core.
Point one was reported by Thomas Kellerer, the other two detected by
code-reading.
Point two is much the nastiest of these issues from an impact standpoint,
though fortunately it affects only a minority of users. The Windows issue
will definitely bite people, but it seems quite unlikely that there would
be undetected corruption from that.
In addition, I failed to resist the temptation to do some minor cosmetic
adjustments, mostly improving the comments.
It would be a good idea to try to improve the error reporting here, but
that seems like material for a separate patch.
Discussion: <nsjrbh$8li$1(at)blaine(dot)gmane(dot)org>
Branch
------
REL9_6_STABLE
Details
-------
http://git.postgresql.org/pg/commitdiff/b6d90607364c4ad9315e28f76c836de4751beca5
Modified Files
--------------
src/bin/pg_upgrade/file.c | 91 +++++++++++++++++++++++++----------------------
1 file changed, 49 insertions(+), 42 deletions(-)
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2016-10-01 01:32:48 | pgsql: Set log_line_prefix and application name in test drivers |
Previous Message | Peter Eisentraut | 2016-09-30 19:29:38 | Re: pgsql: Separate enum from struct |