On Mon, Apr 6, 2015 at 12:57 PM, Fujii Masao <masao.fu...@gmail.com> wrote: > (1) > It outputs an error message to stdout not stderr. > (2) > The tool name should be added at the head of log message as follows, > but not in pg_rewind. > > pg_basebackup: no target directory specified
Agreed. That's inconsistent. > (3) >> if (datadir_source == NULL && connstr_source == NULL) >> { >> pg_fatal("no source specified (--source-pgdata or >> --source-server)\n"); >> pg_fatal("Try \"%s --help\" for more information.\n", progname); >> exit(1); > > Since the first call of pg_fatal exits with 1, the subsequent pg_fatal and > exit > will never be called. True. > (4) > ISTM that set_pglocale_pgservice() needs to be called, but not in pg_rewind. True. I think that we should consider a call to get_restricted_token() as well now that I look at it. > (5) > printf() is used to output an error in some files, e.g., timeline.c and > parsexlog.c. These printf() should be replaced with pg_log or pg_fatal? On top of that, datapagemap.c has a debug message as well using printf, as well as filemap.c in print_filemap(). I guess that you are working on a patch? If not, you are looking for one? Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers