On Mon, Apr 6, 2015 at 12:57 PM, Fujii Masao <masao.fujii@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