From e191a6900520a28ece9393eec2fdd69f292f12c4 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Mon, 6 Jun 2016 09:51:56 -0400 Subject: [PATCH] pg_upgrade: Don't overwrite existing files. For historical reasons, copyFile and rewriteVisibilityMap took a force argument which was always passed as true, meaning that any existing file should be overwritten. However, it seems much safer to instead fail if a file we need to write already exists. While we're at it, remove the "force" argument altogether, since it was never passed as anything other than true (and now we would never pass it as anything other than false, if we kept it). Noted by Andres Freund during post-commit review of the patch that added rewriteVisibilityMap, commit 7087166a88fe0c04fc6636d0d6d6bea1737fc1fb, but this also changes the behavior when copying files without rewriting them. Patch by Masahiko Sawada. --- src/bin/pg_upgrade/file.c | 16 ++++++++-------- src/bin/pg_upgrade/pg_upgrade.h | 5 ++--- src/bin/pg_upgrade/relfilenode.c | 6 +++--- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c index 9b25dc5b28..4f27ce713f 100644 --- a/src/bin/pg_upgrade/file.c +++ b/src/bin/pg_upgrade/file.c @@ -22,7 +22,7 @@ #ifndef WIN32 -static int copy_file(const char *fromfile, const char *tofile, bool force); +static int copy_file(const char *fromfile, const char *tofile); #else static int win32_pghardlink(const char *src, const char *dst); #endif @@ -34,12 +34,12 @@ static int win32_pghardlink(const char *src, const char *dst); * Copies a relation file from src to dst. */ const char * -copyFile(const char *src, const char *dst, bool force) +copyFile(const char *src, const char *dst) { #ifndef WIN32 - if (copy_file(src, dst, force) == -1) + if (copy_file(src, dst) == -1) #else - if (CopyFile(src, dst, !force) == 0) + if (CopyFile(src, dst, true) == 0) #endif return getErrorText(); else @@ -68,7 +68,7 @@ linkFile(const char *src, const char *dst) #ifndef WIN32 static int -copy_file(const char *srcfile, const char *dstfile, bool force) +copy_file(const char *srcfile, const char *dstfile) { #define COPY_BUF_SIZE (50 * BLCKSZ) @@ -87,7 +87,7 @@ copy_file(const char *srcfile, const char *dstfile, bool force) if ((src_fd = open(srcfile, O_RDONLY, 0)) < 0) return -1; - if ((dest_fd = open(dstfile, O_RDWR | O_CREAT | (force ? 0 : O_EXCL), S_IRUSR | S_IWUSR)) < 0) + if ((dest_fd = open(dstfile, O_RDWR | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR)) < 0) { save_errno = errno; @@ -159,7 +159,7 @@ copy_file(const char *srcfile, const char *dstfile, bool force) * VACUUM. */ const char * -rewriteVisibilityMap(const char *fromfile, const char *tofile, bool force) +rewriteVisibilityMap(const char *fromfile, const char *tofile) { int src_fd = 0; int dst_fd = 0; @@ -186,7 +186,7 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile, bool force) return getErrorText(); } - if ((dst_fd = open(tofile, O_RDWR | O_CREAT | (force ? 0 : O_EXCL), S_IRUSR | S_IWUSR)) < 0) + if ((dst_fd = open(tofile, O_RDWR | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR)) < 0) { close(src_fd); return getErrorText(); diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index 5b0014210f..10182de949 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -367,10 +367,9 @@ bool pid_lock_file_exists(const char *datadir); /* file.c */ -const char *copyFile(const char *src, const char *dst, bool force); +const char *copyFile(const char *src, const char *dst); const char *linkFile(const char *src, const char *dst); -const char *rewriteVisibilityMap(const char *fromfile, const char *tofile, - bool force); +const char *rewriteVisibilityMap(const char *fromfile, const char *tofile); void check_hard_link(void); FILE *fopen_priv(const char *path, const char *mode); diff --git a/src/bin/pg_upgrade/relfilenode.c b/src/bin/pg_upgrade/relfilenode.c index 0c1a8220bb..85cb717f74 100644 --- a/src/bin/pg_upgrade/relfilenode.c +++ b/src/bin/pg_upgrade/relfilenode.c @@ -248,9 +248,9 @@ transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_fro /* Rewrite visibility map if needed */ if (vm_must_add_frozenbit && (strcmp(type_suffix, "_vm") == 0)) - msg = rewriteVisibilityMap(old_file, new_file, true); + msg = rewriteVisibilityMap(old_file, new_file); else - msg = copyFile(old_file, new_file, true); + msg = copyFile(old_file, new_file); if (msg) pg_fatal("error while copying relation \"%s.%s\" (\"%s\" to \"%s\"): %s\n", @@ -262,7 +262,7 @@ transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_fro /* Rewrite visibility map if needed */ if (vm_must_add_frozenbit && (strcmp(type_suffix, "_vm") == 0)) - msg = rewriteVisibilityMap(old_file, new_file, true); + msg = rewriteVisibilityMap(old_file, new_file); else msg = linkFile(old_file, new_file);