• Tom Lane's avatar
    Fix multiple portability issues in pg_upgrade's rewriteVisibilityMap(). · 5afcd2aa
    Tom Lane authored
    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@blaine.gmane.org>
    5afcd2aa
file.c 8.07 KB