On 01/06/2015 09:13 AM, Michael Paquier wrote:
Some more comments:
- Nitpicking: the header formats of filemap.c, datapagemap.c,
datapagemap.h and util.h are incorrect (I pushed a fix about that in
pg_rewind itself, feel free to pick it up).

Ah, fixed.

- parsexlog.c has a copyright mention to Nippon Telegraph and
Telephone Corporation. Cannot we drop it safely?

Removed. The file used to contain code for handling different WAL record types that was originally copied from pg_lesslog, hence the NTT copyright. However, that code is no longer there.

- Error codes needs to be generated before building pg_rewind. If I do
for example a simple configure followed by make I get a failure:
$ ./configure
$ cd contrib/pg_rewind && make
In file included from parsexlog.c:16:
In file included from ../../src/include/postgres.h:48:
../../src/include/utils/elog.h:69:10: fatal error: 'utils/errcodes.h'
file not found
#include "utils/errcodes.h"

Many other contrib modules have the same problem. And other subdirectories too. It's not something that this patch needs to fix.

- MSVC build is not supported yet. You need to do something similar to
pg_xlogdump, aka some magic with for example xlogreader.c.
- Build fails with MinGW as there is visibly some unportable code:

Fixed all the errors I got on MSVC. The biggest change was rewriting the code that determines if a file is a relation file, based on its filename. It used a regular expression, which I replaced with a bunch of sscanf calls, and a cross-check that GetRelationPath() returns the same filename.

copy_fetch.c: In function 'check_samefile':
copy_fetch.c:298:2: warning: passing argument 2 of '_fstat64i32' from incompatib
le pointer type [enabled by default]
   if (fstat(fd1, &statbuf1) < 0)
   ^
In file included from ../../src/include/port.h:283:0,
                  from ../../src/include/c.h:1050,
                  from ../../src/include/postgres_fe.h:25,
                  from copy_fetch.c:10:
c:\mingw\include\sys\stat.h:200:32: note: expected 'struct _stat64i32 *' but arg
ument is of type 'struct stat *'
  __CRT_MAYBE_INLINE int __cdecl _fstat64i32(int desc, struct _stat64i32 *_stat)
{

Strange. There isn't anything special about the fstat() calls in pg_rewind. Do you get these from other modules that call fstat, e.g. pg_stat_statements?

I did not see these warnings when building with MSVC, and don't have MinGW installed currently.

Hm. I think that this is something we should try to fix first
upstream.

Yeah, possibly. But here's a new patch anyway.

- Heikki

Attachment: pg_rewind-contrib-4.patch.gz
Description: application/gzip

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to