On Mon, Apr 6, 2015 at 9:10 PM, Fujii Masao <[email protected]> wrote:
> I'm not familiar with native language support (sorry), but don't we need to
> add the shortcut of gettext into every calls of pg_log and pg_fatal, e.g.,
> change pg_fatal("xxx") to pg_fatal(_("xxx"))? I know that fprintf() in
> pg_Log_v() has such shortcut, but I'm not sure if that's enough or not.
I think I addressed those things...
> case PG_FATAL:
> - printf("\n%s", _(message));
> - printf("%s", _("Failure, exiting\n"));
> + fprintf(stderr, _("\n%s: fatal: %s\n"), progname, message);
> + fprintf(stderr, _("Failure, exiting\n"));
>
> Why do we need to output the error level like fatal? This seems also
> inconsistent with the log message policy of other tools.
initdb and psql do not for a couple of warning messages, but perhaps I
am just taking consistency with the original code too seriously.
> Why do we need to output \n at the head of the message?
> The second message "Failure, exiting" is really required?
I think that those things were here to highlight the fact that this is
a fatal bailout, but the error code would help the same way I guess...
>> I eliminated a bunch of
>> newlines in the log messages that seemed really unnecessary to me,
>> simplifying a bit the whole. While hacking this stuff, I noticed as
>> well that pg_rewind could be called as root on non-Windows platform,
>> that's dangerous from a security point of view as process manipulates
>> files in PGDATA. Hence let's block that. On Windows, a restricted
>> token should be used.
>
> Good catch! I think it's better to implement it as a separate patch
> because it's very different from log message problem.
Attached are new patches:
- 0001 fixes the restriction issues: restricted token on Windows, and
forbid non-root user on non-Windows.
- 0002 includes the improvements for logging, addressing the comments above.
Regards,
--
Michael
From 6fa9c9d427352a01d589ce1871b6adecd88cf49c Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Tue, 7 Apr 2015 11:21:17 +0900
Subject: [PATCH 1/2] Fix process handling of pg_rewind
To begin with, pg_rewind should not be allowed to run as root on
non-Windows platforms as it manipulates data folders, and file permissions.
On Windows platforms, it can run under a user that has Administrator rights
but in this case a restricted token needs to be used.
---
src/bin/pg_rewind/pg_rewind.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index dda3a79..200e001 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -24,6 +24,7 @@
#include "access/xlog_internal.h"
#include "catalog/catversion.h"
#include "catalog/pg_control.h"
+#include "common/restricted_token.h"
#include "getopt_long.h"
#include "storage/bufpage.h"
@@ -174,6 +175,21 @@ main(int argc, char **argv)
exit(1);
}
+ /*
+ * Don't allow pg_rewind to be run as root, to avoid overwriting the
+ * ownership of files in the data directory. We need only check for root
+ * -- any other user won't have sufficient permissions to modify files in
+ * the data directory.
+ */
+#ifndef WIN32
+ if (geteuid() == 0)
+ pg_fatal("cannot be executed by \"root\"\n"
+ "You must run %s as the PostgreSQL superuser.\n",
+ progname);
+#endif
+
+ get_restricted_token(progname);
+
/* Connect to remote server */
if (connstr_source)
libpqConnect(connstr_source);
--
2.3.5
From 79103f93393e34e1a795001afd3f43a3a8201500 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Mon, 6 Apr 2015 17:18:21 +0900
Subject: [PATCH 2/2] Fix inconsistent handling of logs in pg_rewind
pg_rewind was handling a couple of things differently compared to the
other src/bin utilities:
- Logging output needs to be flushed on stderr, not stdout
- Logging messages should be prefixed with the application name
- pg_fatal exits with error code of 1, but it was sometimes followed by
subsequent logs, making this information lost in the process.
---
src/bin/pg_rewind/copy_fetch.c | 24 ++++++++---------
src/bin/pg_rewind/datapagemap.c | 4 ++-
src/bin/pg_rewind/file_ops.c | 30 +++++++++++-----------
src/bin/pg_rewind/filemap.c | 16 ++++++------
src/bin/pg_rewind/libpq_fetch.c | 52 ++++++++++++++++++-------------------
src/bin/pg_rewind/logging.c | 13 +++++-----
src/bin/pg_rewind/logging.h | 2 +-
src/bin/pg_rewind/parsexlog.c | 20 +++++++--------
src/bin/pg_rewind/pg_rewind.c | 57 ++++++++++++++++++++++-------------------
src/bin/pg_rewind/pg_rewind.h | 2 ++
src/bin/pg_rewind/timeline.c | 27 +++++++------------
11 files changed, 122 insertions(+), 125 deletions(-)
diff --git a/src/bin/pg_rewind/copy_fetch.c b/src/bin/pg_rewind/copy_fetch.c
index 887fec9..d3430e5 100644
--- a/src/bin/pg_rewind/copy_fetch.c
+++ b/src/bin/pg_rewind/copy_fetch.c
@@ -58,7 +58,7 @@ recurse_dir(const char *datadir, const char *parentpath,
xldir = opendir(fullparentpath);
if (xldir == NULL)
- pg_fatal("could not open directory \"%s\": %s\n",
+ pg_fatal("could not open directory \"%s\": %s",
fullparentpath, strerror(errno));
while (errno = 0, (xlde = readdir(xldir)) != NULL)
@@ -75,7 +75,7 @@ recurse_dir(const char *datadir, const char *parentpath,
if (lstat(fullpath, &fst) < 0)
{
- pg_log(PG_WARNING, "could not stat file \"%s\": %s",
+ pg_log(PG_STATUS, "could not stat file \"%s\": %s",
fullpath, strerror(errno));
/*
@@ -113,13 +113,13 @@ recurse_dir(const char *datadir, const char *parentpath,
len = readlink(fullpath, link_target, sizeof(link_target) - 1);
if (len == -1)
- pg_fatal("readlink() failed on \"%s\": %s\n",
+ pg_fatal("readlink() failed on \"%s\": %s",
fullpath, strerror(errno));
if (len == sizeof(link_target) - 1)
{
/* path was truncated */
- pg_fatal("symbolic link \"%s\" target path too long\n",
+ pg_fatal("symbolic link \"%s\" target path too long",
fullpath);
}
@@ -132,18 +132,18 @@ recurse_dir(const char *datadir, const char *parentpath,
if (strcmp(parentpath, "pg_tblspc") == 0)
recurse_dir(datadir, path, callback);
#else
- pg_fatal("\"%s\" is a symbolic link, but symbolic links are not supported on this platform\n",
+ pg_fatal("\"%s\" is a symbolic link, but symbolic links are not supported on this platform",
fullpath);
#endif /* HAVE_READLINK */
}
}
if (errno)
- pg_fatal("could not read directory \"%s\": %s\n",
+ pg_fatal("could not read directory \"%s\": %s",
fullparentpath, strerror(errno));
if (closedir(xldir))
- pg_fatal("could not close archive location \"%s\": %s\n",
+ pg_fatal("could not close archive location \"%s\": %s",
fullparentpath, strerror(errno));
}
@@ -163,11 +163,11 @@ copy_file_range(const char *path, off_t begin, off_t end, bool trunc)
srcfd = open(srcpath, O_RDONLY | PG_BINARY, 0);
if (srcfd < 0)
- pg_fatal("could not open source file \"%s\": %s\n",
+ pg_fatal("could not open source file \"%s\": %s",
srcpath, strerror(errno));
if (lseek(srcfd, begin, SEEK_SET) == -1)
- pg_fatal("could not seek in source file: %s\n", strerror(errno));
+ pg_fatal("could not seek in source file: %s", strerror(errno));
open_target_file(path, trunc);
@@ -184,17 +184,17 @@ copy_file_range(const char *path, off_t begin, off_t end, bool trunc)
readlen = read(srcfd, buf, len);
if (readlen < 0)
- pg_fatal("could not read file \"%s\": %s\n",
+ pg_fatal("could not read file \"%s\": %s",
srcpath, strerror(errno));
else if (readlen == 0)
- pg_fatal("unexpected EOF while reading file \"%s\"\n", srcpath);
+ pg_fatal("unexpected EOF while reading file \"%s\"", srcpath);
write_target_range(buf, begin, readlen);
begin += readlen;
}
if (close(srcfd) != 0)
- pg_fatal("error closing file \"%s\": %s\n", srcpath, strerror(errno));
+ pg_fatal("error closing file \"%s\": %s", srcpath, strerror(errno));
}
/*
diff --git a/src/bin/pg_rewind/datapagemap.c b/src/bin/pg_rewind/datapagemap.c
index 3477366..8814954 100644
--- a/src/bin/pg_rewind/datapagemap.c
+++ b/src/bin/pg_rewind/datapagemap.c
@@ -13,6 +13,8 @@
#include "postgres_fe.h"
#include "datapagemap.h"
+#include "pg_rewind.h"
+#include "logging.h"
struct datapagemap_iterator
{
@@ -120,7 +122,7 @@ datapagemap_print(datapagemap_t *map)
iter = datapagemap_iterate(map);
while (datapagemap_next(iter, &blocknum))
- printf(" blk %u\n", blocknum);
+ pg_log(PG_DEBUG, "block number %u", blocknum);
free(iter);
}
diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c
index 589a01a..4620064 100644
--- a/src/bin/pg_rewind/file_ops.c
+++ b/src/bin/pg_rewind/file_ops.c
@@ -61,7 +61,7 @@ open_target_file(const char *path, bool trunc)
mode |= O_TRUNC;
dstfd = open(dstpath, mode, 0600);
if (dstfd < 0)
- pg_fatal("could not open destination file \"%s\": %s\n",
+ pg_fatal("could not open destination file \"%s\": %s",
dstpath, strerror(errno));
}
@@ -75,7 +75,7 @@ close_target_file(void)
return;
if (close(dstfd) != 0)
- pg_fatal("error closing destination file \"%s\": %s\n",
+ pg_fatal("error closing destination file \"%s\": %s",
dstpath, strerror(errno));
dstfd = -1;
@@ -96,7 +96,7 @@ write_target_range(char *buf, off_t begin, size_t size)
return;
if (lseek(dstfd, begin, SEEK_SET) == -1)
- pg_fatal("could not seek in destination file \"%s\": %s\n",
+ pg_fatal("could not seek in destination file \"%s\": %s",
dstpath, strerror(errno));
writeleft = size;
@@ -107,7 +107,7 @@ write_target_range(char *buf, off_t begin, size_t size)
writelen = write(dstfd, p, writeleft);
if (writelen < 0)
- pg_fatal("could not write file \"%s\": %s\n",
+ pg_fatal("could not write file \"%s\": %s",
dstpath, strerror(errno));
p += writelen;
@@ -156,7 +156,7 @@ create_target(file_entry_t *entry)
case FILE_TYPE_REGULAR:
/* can't happen. Regular files are created with open_target_file. */
- pg_fatal("invalid action (CREATE) for regular file\n");
+ pg_fatal("invalid action (CREATE) for regular file");
break;
}
}
@@ -171,7 +171,7 @@ remove_target_file(const char *path)
snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path);
if (unlink(dstpath) != 0)
- pg_fatal("could not remove file \"%s\": %s\n",
+ pg_fatal("could not remove file \"%s\": %s",
dstpath, strerror(errno));
}
@@ -188,11 +188,11 @@ truncate_target_file(const char *path, off_t newsize)
fd = open(dstpath, O_WRONLY, 0);
if (fd < 0)
- pg_fatal("could not open file \"%s\" for truncation: %s\n",
+ pg_fatal("could not open file \"%s\" for truncation: %s",
dstpath, strerror(errno));
if (ftruncate(fd, newsize) != 0)
- pg_fatal("could not truncate file \"%s\" to %u bytes: %s\n",
+ pg_fatal("could not truncate file \"%s\" to %u bytes: %s",
dstpath, (unsigned int) newsize, strerror(errno));
close(fd);
@@ -208,7 +208,7 @@ create_target_dir(const char *path)
snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path);
if (mkdir(dstpath, S_IRWXU) != 0)
- pg_fatal("could not create directory \"%s\": %s\n",
+ pg_fatal("could not create directory \"%s\": %s",
dstpath, strerror(errno));
}
@@ -222,7 +222,7 @@ remove_target_dir(const char *path)
snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path);
if (rmdir(dstpath) != 0)
- pg_fatal("could not remove directory \"%s\": %s\n",
+ pg_fatal("could not remove directory \"%s\": %s",
dstpath, strerror(errno));
}
@@ -236,7 +236,7 @@ create_target_symlink(const char *path, const char *link)
snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path);
if (symlink(link, dstpath) != 0)
- pg_fatal("could not create symbolic link at \"%s\": %s\n",
+ pg_fatal("could not create symbolic link at \"%s\": %s",
dstpath, strerror(errno));
}
@@ -250,7 +250,7 @@ remove_target_symlink(const char *path)
snprintf(dstpath, sizeof(dstpath), "%s/%s", datadir_target, path);
if (unlink(dstpath) != 0)
- pg_fatal("could not remove symbolic link \"%s\": %s\n",
+ pg_fatal("could not remove symbolic link \"%s\": %s",
dstpath, strerror(errno));
}
@@ -280,11 +280,11 @@ slurpFile(const char *datadir, const char *path, size_t *filesize)
snprintf(fullpath, sizeof(fullpath), "%s/%s", datadir, path);
if ((fd = open(fullpath, O_RDONLY | PG_BINARY, 0)) == -1)
- pg_fatal("could not open file \"%s\" for reading: %s\n",
+ pg_fatal("could not open file \"%s\" for reading: %s",
fullpath, strerror(errno));
if (fstat(fd, &statbuf) < 0)
- pg_fatal("could not open file \"%s\" for reading: %s\n",
+ pg_fatal("could not open file \"%s\" for reading: %s",
fullpath, strerror(errno));
len = statbuf.st_size;
@@ -292,7 +292,7 @@ slurpFile(const char *datadir, const char *path, size_t *filesize)
buffer = pg_malloc(len + 1);
if (read(fd, buffer, len) != len)
- pg_fatal("could not read file \"%s\": %s\n",
+ pg_fatal("could not read file \"%s\": %s",
fullpath, strerror(errno));
close(fd);
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index ee6e6db..8788335 100644
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -93,7 +93,7 @@ process_remote_file(const char *path, file_type_t type, size_t newsize,
* regular file
*/
if (type != FILE_TYPE_REGULAR && isRelDataFile(path))
- pg_fatal("data file in source \"%s\" is not a regular file\n", path);
+ pg_fatal("data file in source \"%s\" is not a regular file", path);
snprintf(localpath, sizeof(localpath), "%s/%s", datadir_target, path);
@@ -101,7 +101,7 @@ process_remote_file(const char *path, file_type_t type, size_t newsize,
if (lstat(localpath, &statbuf) < 0)
{
if (errno != ENOENT)
- pg_fatal("could not stat file \"%s\": %s\n",
+ pg_fatal("could not stat file \"%s\": %s",
localpath, strerror(errno));
exists = false;
@@ -115,7 +115,7 @@ process_remote_file(const char *path, file_type_t type, size_t newsize,
if (exists && !S_ISDIR(statbuf.st_mode))
{
/* it's a directory in target, but not in source. Strange.. */
- pg_fatal("\"%s\" is not a directory\n", localpath);
+ pg_fatal("\"%s\" is not a directory", localpath);
}
if (!exists)
@@ -138,7 +138,7 @@ process_remote_file(const char *path, file_type_t type, size_t newsize,
* It's a symbolic link in target, but not in source.
* Strange..
*/
- pg_fatal("\"%s\" is not a symbolic link\n", localpath);
+ pg_fatal("\"%s\" is not a symbolic link", localpath);
}
if (!exists)
@@ -150,7 +150,7 @@ process_remote_file(const char *path, file_type_t type, size_t newsize,
case FILE_TYPE_REGULAR:
if (exists && !S_ISREG(statbuf.st_mode))
- pg_fatal("\"%s\" is not a regular file\n", localpath);
+ pg_fatal("\"%s\" is not a regular file", localpath);
if (!exists || !isRelDataFile(path))
{
@@ -266,7 +266,7 @@ process_local_file(const char *path, file_type_t type, size_t oldsize,
if (map->nlist == 0)
{
/* should not happen */
- pg_fatal("remote file list is empty\n");
+ pg_fatal("remote file list is empty");
}
filemap_list_to_array(map);
@@ -381,7 +381,7 @@ process_block_change(ForkNumber forknum, RelFileNode rnode, BlockNumber blkno)
break;
case FILE_ACTION_CREATE:
- pg_fatal("unexpected page modification for directory or symbolic link \"%s\"\n", entry->path);
+ pg_fatal("unexpected page modification for directory or symbolic link \"%s\"", entry->path);
}
}
else
@@ -514,7 +514,7 @@ print_filemap(void)
if (entry->action != FILE_ACTION_NONE ||
entry->pagemap.bitmapsize > 0)
{
- printf("%s (%s)\n", entry->path, action_to_str(entry->action));
+ pg_log(PG_DEBUG, "%s (%s)", entry->path, action_to_str(entry->action));
if (entry->pagemap.bitmapsize > 0)
datapagemap_print(&entry->pagemap);
diff --git a/src/bin/pg_rewind/libpq_fetch.c b/src/bin/pg_rewind/libpq_fetch.c
index 23c971a..b7accba 100644
--- a/src/bin/pg_rewind/libpq_fetch.c
+++ b/src/bin/pg_rewind/libpq_fetch.c
@@ -52,10 +52,10 @@ libpqConnect(const char *connstr)
conn = PQconnectdb(connstr);
if (PQstatus(conn) == CONNECTION_BAD)
- pg_fatal("could not connect to remote server: %s\n",
+ pg_fatal("could not connect to remote server: %s",
PQerrorMessage(conn));
- pg_log(PG_PROGRESS, "connected to remote server\n");
+ pg_log(PG_PROGRESS, "connected to remote server");
/*
* Check that the server is not in hot standby mode. There is no
@@ -65,7 +65,7 @@ libpqConnect(const char *connstr)
*/
str = run_simple_query("SELECT pg_is_in_recovery()");
if (strcmp(str, "f") != 0)
- pg_fatal("source server must not be in recovery mode\n");
+ pg_fatal("source server must not be in recovery mode");
pg_free(str);
/*
@@ -75,7 +75,7 @@ libpqConnect(const char *connstr)
*/
str = run_simple_query("SHOW full_page_writes");
if (strcmp(str, "on") != 0)
- pg_fatal("full_page_writes must be enabled in the source server\n");
+ pg_fatal("full_page_writes must be enabled in the source server");
pg_free(str);
}
@@ -91,12 +91,12 @@ run_simple_query(const char *sql)
res = PQexec(conn, sql);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pg_fatal("error running query (%s) in source server: %s\n",
+ pg_fatal("error running query (%s) in source server: %s",
sql, PQresultErrorMessage(res));
/* sanity check the result set */
if (PQnfields(res) != 1 || PQntuples(res) != 1 || PQgetisnull(res, 0, 0))
- pg_fatal("unexpected result set while running query\n");
+ pg_fatal("unexpected result set while running query");
result = pg_strdup(PQgetvalue(res, 0, 0));
@@ -119,7 +119,7 @@ libpqGetCurrentXlogInsertLocation(void)
val = run_simple_query("SELECT pg_current_xlog_insert_location()");
if (sscanf(val, "%X/%X", &hi, &lo) != 2)
- pg_fatal("unexpected result \"%s\" while fetching current XLOG insert location\n", val);
+ pg_fatal("unexpected result \"%s\" while fetching current XLOG insert location", val);
result = ((uint64) hi) << 32 | lo;
@@ -167,12 +167,12 @@ libpqProcessFileList(void)
res = PQexec(conn, sql);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pg_fatal("unexpected result while fetching file list: %s\n",
+ pg_fatal("unexpected result while fetching file list: %s",
PQresultErrorMessage(res));
/* sanity check the result set */
if (PQnfields(res) != 4)
- pg_fatal("unexpected result set while fetching file list\n");
+ pg_fatal("unexpected result set while fetching file list");
/* Read result to local variables */
for (i = 0; i < PQntuples(res); i++)
@@ -210,12 +210,12 @@ receiveFileChunks(const char *sql)
PGresult *res;
if (PQsendQueryParams(conn, sql, 0, NULL, NULL, NULL, NULL, 1) != 1)
- pg_fatal("could not send query: %s\n", PQerrorMessage(conn));
+ pg_fatal("could not send query: %s", PQerrorMessage(conn));
pg_log(PG_DEBUG, "getting file chunks");
if (PQsetSingleRowMode(conn) != 1)
- pg_fatal("could not set libpq connection to single row mode\n");
+ pg_fatal("could not set libpq connection to single row mode");
while ((res = PQgetResult(conn)) != NULL)
{
@@ -234,19 +234,19 @@ receiveFileChunks(const char *sql)
continue; /* final zero-row result */
default:
- pg_fatal("unexpected result while fetching remote files: %s\n",
+ pg_fatal("unexpected result while fetching remote files: %s",
PQresultErrorMessage(res));
}
/* sanity check the result set */
if (PQnfields(res) != 3 || PQntuples(res) != 1)
- pg_fatal("unexpected result set size while fetching remote files\n");
+ pg_fatal("unexpected result set size while fetching remote files");
if (PQftype(res, 0) != TEXTOID &&
PQftype(res, 1) != INT4OID &&
PQftype(res, 2) != BYTEAOID)
{
- pg_fatal("unexpected data types in result set while fetching remote files: %u %u %u\n",
+ pg_fatal("unexpected data types in result set while fetching remote files: %u %u %u",
PQftype(res, 0), PQftype(res, 1), PQftype(res, 2));
}
@@ -254,18 +254,18 @@ receiveFileChunks(const char *sql)
PQfformat(res, 1) != 1 &&
PQfformat(res, 2) != 1)
{
- pg_fatal("unexpected result format while fetching remote files\n");
+ pg_fatal("unexpected result format while fetching remote files");
}
if (PQgetisnull(res, 0, 0) ||
PQgetisnull(res, 0, 1) ||
PQgetisnull(res, 0, 2))
{
- pg_fatal("unexpected NULL result while fetching remote files\n");
+ pg_fatal("unexpected NULL result while fetching remote files");
}
if (PQgetlength(res, 0, 1) != sizeof(int32))
- pg_fatal("unexpected result length while fetching remote files\n");
+ pg_fatal("unexpected result length while fetching remote files");
/* Read result set to local variables */
memcpy(&chunkoff, PQgetvalue(res, 0, 1), sizeof(int32));
@@ -279,7 +279,7 @@ receiveFileChunks(const char *sql)
chunk = PQgetvalue(res, 0, 2);
- pg_log(PG_DEBUG, "received chunk for file \"%s\", off %d, len %d\n",
+ pg_log(PG_DEBUG, "received chunk for file \"%s\", off %d, len %d",
filename, chunkoff, chunksize);
open_target_file(filename, false);
@@ -308,12 +308,12 @@ libpqGetFile(const char *filename, size_t *filesize)
1, NULL, paramValues, NULL, NULL, 1);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
- pg_fatal("unexpected result while fetching remote file \"%s\": %s\n",
+ pg_fatal("unexpected result while fetching remote file \"%s\": %s",
filename, PQresultErrorMessage(res));
/* sanity check the result set */
if (PQntuples(res) != 1 || PQgetisnull(res, 0, 0))
- pg_fatal("unexpected result set while fetching remote file \"%s\"\n",
+ pg_fatal("unexpected result set while fetching remote file \"%s\"",
filename);
/* Read result to local variables */
@@ -322,7 +322,7 @@ libpqGetFile(const char *filename, size_t *filesize)
memcpy(result, PQgetvalue(res, 0, 0), len);
result[len] = '\0';
- pg_log(PG_DEBUG, "fetched file \"%s\", length %d\n", filename, len);
+ pg_log(PG_DEBUG, "fetched file \"%s\", length %d", filename, len);
if (filesize)
*filesize = len;
@@ -354,7 +354,7 @@ fetch_file_range(const char *path, unsigned int begin, unsigned int end)
snprintf(linebuf, sizeof(linebuf), "%s\t%u\t%u\n", path, begin, len);
if (PQputCopyData(conn, linebuf, strlen(linebuf)) != 1)
- pg_fatal("error sending COPY data: %s\n",
+ pg_fatal("error sending COPY data: %s",
PQerrorMessage(conn));
begin += len;
@@ -380,14 +380,14 @@ libpq_executeFileMap(filemap_t *map)
res = PQexec(conn, sql);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
- pg_fatal("error creating temporary table: %s\n",
+ pg_fatal("error creating temporary table: %s",
PQresultErrorMessage(res));
sql = "COPY fetchchunks FROM STDIN";
res = PQexec(conn, sql);
if (PQresultStatus(res) != PGRES_COPY_IN)
- pg_fatal("unexpected result while sending file list: %s\n",
+ pg_fatal("unexpected result while sending file list: %s",
PQresultErrorMessage(res));
for (i = 0; i < map->narray; i++)
@@ -428,13 +428,13 @@ libpq_executeFileMap(filemap_t *map)
}
if (PQputCopyEnd(conn, NULL) != 1)
- pg_fatal("error sending end-of-COPY: %s\n",
+ pg_fatal("error sending end-of-COPY: %s",
PQerrorMessage(conn));
while ((res = PQgetResult(conn)) != NULL)
{
if (PQresultStatus(res) != PGRES_COMMAND_OK)
- pg_fatal("unexpected result while sending file list: %s\n",
+ pg_fatal("unexpected result while sending file list: %s",
PQresultErrorMessage(res));
}
diff --git a/src/bin/pg_rewind/logging.c b/src/bin/pg_rewind/logging.c
index aba12d8..eb6c103 100644
--- a/src/bin/pg_rewind/logging.c
+++ b/src/bin/pg_rewind/logging.c
@@ -40,28 +40,27 @@ pg_log_v(eLogType type, const char *fmt, va_list ap)
{
case PG_DEBUG:
if (debug)
- printf("%s", _(message));
+ fprintf(stderr, "%s: %s\n", progname, _(message));
break;
case PG_PROGRESS:
if (showprogress)
- printf("%s", _(message));
+ fprintf(stderr, "%s: %s\n", progname, _(message));
break;
- case PG_WARNING:
- printf("%s", _(message));
+ case PG_STATUS:
+ fprintf(stderr, "%s: %s\n", progname, _(message));
break;
case PG_FATAL:
- printf("\n%s", _(message));
- printf("%s", _("Failure, exiting\n"));
+ fprintf(stderr, "%s: %s\n", progname, _(message));
exit(1);
break;
default:
break;
}
- fflush(stdout);
+ fflush(stderr);
}
diff --git a/src/bin/pg_rewind/logging.h b/src/bin/pg_rewind/logging.h
index 0272a22..76fd367 100644
--- a/src/bin/pg_rewind/logging.h
+++ b/src/bin/pg_rewind/logging.h
@@ -23,7 +23,7 @@ typedef enum
{
PG_DEBUG,
PG_PROGRESS,
- PG_WARNING,
+ PG_STATUS,
PG_FATAL
} eLogType;
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index 3cf96ab..9df1a5a 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -84,11 +84,11 @@ extractPageMap(const char *datadir, XLogRecPtr startpoint, TimeLineID tli,
errptr = startpoint ? startpoint : xlogreader->EndRecPtr;
if (errormsg)
- pg_fatal("error reading WAL at %X/%X: %s\n",
+ pg_fatal("error reading WAL at %X/%X: %s",
(uint32) (errptr >> 32), (uint32) (errptr),
errormsg);
else
- pg_fatal("error reading WAL at %X/%X\n",
+ pg_fatal("error reading WAL at %X/%X",
(uint32) (startpoint >> 32),
(uint32) (startpoint));
}
@@ -130,10 +130,10 @@ readOneRecord(const char *datadir, XLogRecPtr ptr, TimeLineID tli)
if (record == NULL)
{
if (errormsg)
- pg_fatal("could not read WAL record at %X/%X: %s\n",
+ pg_fatal("could not read WAL record at %X/%X: %s",
(uint32) (ptr >> 32), (uint32) (ptr), errormsg);
else
- pg_fatal("could not read WAL record at %X/%X\n",
+ pg_fatal("could not read WAL record at %X/%X",
(uint32) (ptr >> 32), (uint32) (ptr));
}
endptr = xlogreader->EndRecPtr;
@@ -188,11 +188,11 @@ findLastCheckpoint(const char *datadir, XLogRecPtr forkptr, TimeLineID tli,
if (record == NULL)
{
if (errormsg)
- pg_fatal("could not find previous WAL record at %X/%X: %s\n",
+ pg_fatal("could not find previous WAL record at %X/%X: %s",
(uint32) (searchptr >> 32), (uint32) (searchptr),
errormsg);
else
- pg_fatal("could not find previous WAL record at %X/%X\n",
+ pg_fatal("could not find previous WAL record at %X/%X",
(uint32) (searchptr >> 32), (uint32) (searchptr));
}
@@ -265,7 +265,7 @@ SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
if (xlogreadfd < 0)
{
- printf(_("could not open file \"%s\": %s\n"), xlogfpath,
+ pg_log(PG_STATUS, "could not open file \"%s\": %s", xlogfpath,
strerror(errno));
return -1;
}
@@ -279,14 +279,14 @@ SimpleXLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr,
/* Read the requested page */
if (lseek(xlogreadfd, (off_t) targetPageOff, SEEK_SET) < 0)
{
- printf(_("could not seek in file \"%s\": %s\n"), xlogfpath,
+ pg_log(PG_STATUS, "could not seek in file \"%s\": %s", xlogfpath,
strerror(errno));
return -1;
}
if (read(xlogreadfd, readBuf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
{
- printf(_("could not read from file \"%s\": %s\n"), xlogfpath,
+ pg_log(PG_STATUS, "could not read from file \"%s\": %s", xlogfpath,
strerror(errno));
return -1;
}
@@ -357,7 +357,7 @@ extractPageInfo(XLogReaderState *record)
* track that change.
*/
pg_fatal("WAL record modifies a relation, but record type is not recognized\n"
- "lsn: %X/%X, rmgr: %s, info: %02X\n",
+ "lsn: %X/%X, rmgr: %s, info: %02X",
(uint32) (record->ReadRecPtr >> 32), (uint32) (record->ReadRecPtr),
RmgrNames[rmid], info);
}
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 200e001..28221f4 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -28,7 +28,7 @@
#include "getopt_long.h"
#include "storage/bufpage.h"
-static void usage(const char *progname);
+static void usage(void);
static void createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli,
XLogRecPtr checkpointloc);
@@ -42,7 +42,7 @@ static void findCommonAncestorTimeline(XLogRecPtr *recptr, TimeLineID *tli);
static ControlFileData ControlFile_target;
static ControlFileData ControlFile_source;
-const char *progname;
+const char *progname = NULL;
/* Configuration options */
char *datadir_target = NULL;
@@ -54,7 +54,7 @@ bool showprogress = false;
bool dry_run = false;
static void
-usage(const char *progname)
+usage(void)
{
printf(_("%s resynchronizes a cluster with another copy of the cluster.\n\n"), progname);
printf(_("Usage:\n %s [OPTION]...\n\n"), progname);
@@ -103,6 +103,7 @@ main(int argc, char **argv)
TimeLineID endtli;
ControlFileData ControlFile_new;
+ set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_rewind"));
progname = get_progname(argv[0]);
/* Process command-line arguments */
@@ -110,7 +111,7 @@ main(int argc, char **argv)
{
if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-?") == 0)
{
- usage(progname);
+ usage();
exit(0);
}
if (strcmp(argv[1], "--version") == 0 || strcmp(argv[1], "-V") == 0)
@@ -156,21 +157,21 @@ main(int argc, char **argv)
/* No source given? Show usage */
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);
+ pg_log(PG_STATUS, "no source specified (--source-pgdata or --source-server)");
+ fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit(1);
}
if (datadir_target == NULL)
{
- pg_fatal("no target data directory specified (--target-pgdata)\n");
+ pg_log(PG_STATUS, "no target data directory specified (--target-pgdata)");
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit(1);
}
if (argc != optind)
{
- pg_fatal("%s: invalid arguments\n", progname);
+ pg_log(PG_STATUS, "invalid arguments");
fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
exit(1);
}
@@ -213,10 +214,11 @@ main(int argc, char **argv)
* do.
*/
if (ControlFile_target.checkPointCopy.ThisTimeLineID == ControlFile_source.checkPointCopy.ThisTimeLineID)
- pg_fatal("source and target cluster are on the same timeline\n");
+ pg_fatal("source and target cluster are on the same timeline");
findCommonAncestorTimeline(&divergerec, &lastcommontli);
- printf(_("The servers diverged at WAL position %X/%X on timeline %u.\n"),
+ pg_log(PG_STATUS,
+ "The servers diverged at WAL position %X/%X on timeline %u.",
(uint32) (divergerec >> 32), (uint32) divergerec, lastcommontli);
/*
@@ -251,13 +253,14 @@ main(int argc, char **argv)
if (!rewind_needed)
{
- printf(_("No rewind required.\n"));
+ pg_log(PG_STATUS, "No rewind required.");
exit(0);
}
findLastCheckpoint(datadir_target, divergerec, lastcommontli,
&chkptrec, &chkpttli, &chkptredo);
- printf(_("Rewinding from last common checkpoint at %X/%X on timeline %u\n"),
+ pg_log(PG_STATUS,
+ "Rewinding from last common checkpoint at %X/%X on timeline %u",
(uint32) (chkptrec >> 32), (uint32) chkptrec,
chkpttli);
@@ -265,9 +268,9 @@ main(int argc, char **argv)
* Build the filemap, by comparing the remote and local data directories.
*/
filemap_create();
- pg_log(PG_PROGRESS, "reading source file list\n");
+ pg_log(PG_PROGRESS, "reading source file list");
fetchRemoteFileList();
- pg_log(PG_PROGRESS, "reading target file list\n");
+ pg_log(PG_PROGRESS, "reading target file list");
traverse_datadir(datadir_target, &process_local_file);
/*
@@ -277,7 +280,7 @@ main(int argc, char **argv)
* XXX: If we supported rewinding a server that was not shut down cleanly,
* we would need to replay until the end of WAL here.
*/
- pg_log(PG_PROGRESS, "reading WAL in target\n");
+ pg_log(PG_PROGRESS, "reading WAL in target");
extractPageMap(datadir_target, chkptrec, lastcommontli,
ControlFile_target.checkPoint);
filemap_finalize();
@@ -294,7 +297,7 @@ main(int argc, char **argv)
*/
if (showprogress)
{
- pg_log(PG_PROGRESS, "Need to copy %lu MB (total source directory size is %lu MB)\n",
+ pg_log(PG_PROGRESS, "Need to copy %lu MB (total source directory size is %lu MB)",
(unsigned long) (filemap->fetch_size / (1024 * 1024)),
(unsigned long) (filemap->total_size / (1024 * 1024)));
@@ -311,7 +314,7 @@ main(int argc, char **argv)
progress_report(true);
- pg_log(PG_PROGRESS, "\ncreating backup label and updating control file\n");
+ pg_log(PG_PROGRESS, "\ncreating backup label and updating control file");
createBackupLabel(chkptredo, chkpttli, chkptrec);
/*
@@ -339,7 +342,7 @@ main(int argc, char **argv)
ControlFile_new.state = DB_IN_ARCHIVE_RECOVERY;
updateControlFile(&ControlFile_new);
- printf(_("Done!\n"));
+ pg_log(PG_STATUS, "Done!");
return 0;
}
@@ -351,7 +354,7 @@ sanityChecks(void)
/* Check system_id match */
if (ControlFile_target.system_identifier != ControlFile_source.system_identifier)
- pg_fatal("source and target clusters are from different systems\n");
+ pg_fatal("source and target clusters are from different systems");
/* check version */
if (ControlFile_target.pg_control_version != PG_CONTROL_VERSION ||
@@ -359,7 +362,7 @@ sanityChecks(void)
ControlFile_target.catalog_version_no != CATALOG_VERSION_NO ||
ControlFile_source.catalog_version_no != CATALOG_VERSION_NO)
{
- pg_fatal("clusters are not compatible with this version of pg_rewind\n");
+ pg_fatal("clusters are not compatible with this version of pg_rewind");
}
/*
@@ -369,7 +372,7 @@ sanityChecks(void)
if (ControlFile_target.data_checksum_version != PG_DATA_CHECKSUM_VERSION &&
!ControlFile_target.wal_log_hints)
{
- pg_fatal("target server need to use either data checksums or \"wal_log_hints = on\"\n");
+ pg_fatal("target server need to use either data checksums or \"wal_log_hints = on\"");
}
/*
@@ -379,7 +382,7 @@ sanityChecks(void)
* as long as it isn't running at the moment.
*/
if (ControlFile_target.state != DB_SHUTDOWNED)
- pg_fatal("target server must be shut down cleanly\n");
+ pg_fatal("target server must be shut down cleanly");
/*
* When the source is a data directory, also require that the source
@@ -387,7 +390,7 @@ sanityChecks(void)
* limitation, but better safe than sorry.
*/
if (datadir_source && ControlFile_source.state != DB_SHUTDOWNED)
- pg_fatal("source data directory must be shut down cleanly\n");
+ pg_fatal("source data directory must be shut down cleanly");
}
/*
@@ -454,7 +457,7 @@ findCommonAncestorTimeline(XLogRecPtr *recptr, TimeLineID *tli)
}
}
- pg_fatal("could not find common ancestor of the source and target cluster's timelines\n");
+ pg_fatal("could not find common ancestor of the source and target cluster's timelines");
}
@@ -494,7 +497,7 @@ createBackupLabel(XLogRecPtr startpoint, TimeLineID starttli, XLogRecPtr checkpo
(uint32) (checkpointloc >> 32), (uint32) checkpointloc,
strfbuf);
if (len >= sizeof(buf))
- pg_fatal("backup label buffer too small\n"); /* shouldn't happen */
+ pg_fatal("backup label buffer too small"); /* shouldn't happen */
/* TODO: move old file out of the way, if any. */
open_target_file("backup_label", true); /* BACKUP_LABEL_FILE */
@@ -516,7 +519,7 @@ checkControlFile(ControlFileData *ControlFile)
/* And simply compare it */
if (!EQ_CRC32C(crc, ControlFile->crc))
- pg_fatal("unexpected control file CRC\n");
+ pg_fatal("unexpected control file CRC");
}
/*
@@ -526,7 +529,7 @@ static void
digestControlFile(ControlFileData *ControlFile, char *src, size_t size)
{
if (size != PG_CONTROL_SIZE)
- pg_fatal("unexpected control file size %d, expected %d\n",
+ pg_fatal("unexpected control file size %d, expected %d",
(int) size, PG_CONTROL_SIZE);
memcpy(ControlFile, src, sizeof(ControlFileData));
diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h
index e281369..ee7ba13 100644
--- a/src/bin/pg_rewind/pg_rewind.h
+++ b/src/bin/pg_rewind/pg_rewind.h
@@ -19,6 +19,8 @@
#include "storage/block.h"
#include "storage/relfilenode.h"
+extern const char *progname;
+
/* Configuration options */
extern char *datadir_target;
extern char *datadir_source;
diff --git a/src/bin/pg_rewind/timeline.c b/src/bin/pg_rewind/timeline.c
index 07ca370..7cdfc50 100644
--- a/src/bin/pg_rewind/timeline.c
+++ b/src/bin/pg_rewind/timeline.c
@@ -9,6 +9,7 @@
*/
#include "postgres_fe.h"
+#include "logging.h"
#include "pg_rewind.h"
#include "access/timeline.h"
@@ -73,22 +74,15 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries)
if (nfields < 1)
{
/* expect a numeric timeline ID as first field of line */
- printf(_("syntax error in history file: %s\n"), fline);
- printf(_("Expected a numeric timeline ID.\n"));
- exit(1);
+ pg_fatal("syntax error in history file: %s\n"
+ "Expected a numeric timeline ID.", fline);
}
if (nfields != 3)
- {
- printf(_("syntax error in history file: %s\n"), fline);
- printf(_("Expected an XLOG switchpoint location.\n"));
- exit(1);
- }
+ pg_fatal("syntax error in history file: %s\n"
+ "Expected an XLOG switchpoint location.", fline);
if (entries && tli <= lasttli)
- {
- printf(_("invalid data in history file: %s\n"), fline);
- printf(_("Timeline IDs must be in increasing sequence.\n"));
- exit(1);
- }
+ pg_fatal("invalid data in history file: %s\n"
+ "Timeline IDs must be in increasing sequence.", fline);
lasttli = tli;
@@ -105,11 +99,8 @@ rewind_parseTimeLineHistory(char *buffer, TimeLineID targetTLI, int *nentries)
}
if (entries && targetTLI <= lasttli)
- {
- printf(_("invalid data in history file\n"));
- printf(_("Timeline IDs must be less than child timeline's ID.\n"));
- exit(1);
- }
+ pg_fatal("invalid data in history file\n"
+ "Timeline IDs must be less than child timeline's ID.");
/*
* Create one more entry for the "tip" of the timeline, which has no entry
--
2.3.5
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
