From f002ed2b8e45286fe73a36e119cba2016ea0de19 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 30 Sep 2016 20:40:27 -0400
Subject: [PATCH] Improve error reporting in pg_upgrade's file
 copying/linking/rewriting.

The previous design for this had copyFile(), linkFile(), and
rewriteVisibilityMap() returning strerror strings, with the caller
producing one-size-fits-all error messages based on that.  This made it
impossible to produce messages that described the failures with any degree
of precision, especially not short-read problems since those don't set
errno at all.

Since pg_upgrade has no intention of continuing after any error in this
area, let's fix this by just letting these functions call pg_fatal() for
themselves, making it easy for each point of failure to have a suitable
error message.  Taking this approach also allows dropping cleanup code
that was unnecessary and was often rather sloppy about preserving errno.
To not lose relevant info that was reported before, pass in the schema name
and table name of the current table so that they can be included in the
error reports.

An additional problem was the use of getErrorText(), which was flat out
wrong for all but a couple of call sites, because it unconditionally did
"_dosmaperr(GetLastError())" on Windows.  That's only appropriate when
reporting an error from a Windows-native API, which only a couple of
the callers were actually doing.  Thus, even the reported strerror string
would be unrelated to the actual failure in many cases on Windows.
To fix, get rid of getErrorText() altogether, and just have call sites
do strerror(errno) instead, since that's the way all the rest of our
frontend programs do it.  Add back the _dosmaperr() calls in the two
places where that's actually appropriate.

In passing, make assorted messages hew more closely to project style
guidelines, notably by removing initial capitals in not-complete-sentence
primary error messages.  (I didn't make any effort to clean up places
I didn't have another reason to touch, though.)

Per discussion of a report from Thomas Kellerer.  Back-patch to 9.6,
but no further; given the relative infrequency of reports of problems
here, it's not clear it's worth adapting the patch to older branches.

Patch by me, but with credit to Alvaro Herrera for spotting the issue
with getErrorText's misuse of _dosmaperr().

Discussion: <nsjrbh$8li$1@blaine.gmane.org>
---
 src/bin/pg_upgrade/check.c       |  32 +++---
 src/bin/pg_upgrade/controldata.c |   4 +-
 src/bin/pg_upgrade/exec.c        |   8 +-
 src/bin/pg_upgrade/file.c        | 182 +++++++++++++------------------
 src/bin/pg_upgrade/function.c    |   6 +-
 src/bin/pg_upgrade/option.c      |   4 +-
 src/bin/pg_upgrade/pg_upgrade.c  |   2 +-
 src/bin/pg_upgrade/pg_upgrade.h  |  11 +-
 src/bin/pg_upgrade/relfilenode.c |  42 +++----
 src/bin/pg_upgrade/tablespace.c  |   4 +-
 src/bin/pg_upgrade/util.c        |  15 ---
 src/bin/pg_upgrade/version.c     |   6 +-
 12 files changed, 132 insertions(+), 184 deletions(-)

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index ed41dee6a54..42bf4992a08 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -431,8 +431,8 @@ create_script_for_cluster_analyze(char **analyze_script_file_name)
 										 SCRIPT_PREFIX, SCRIPT_EXT);
 
 	if ((script = fopen_priv(*analyze_script_file_name, "w")) == NULL)
-		pg_fatal("Could not open file \"%s\": %s\n",
-				 *analyze_script_file_name, getErrorText());
+		pg_fatal("could not open file \"%s\": %s\n",
+				 *analyze_script_file_name, strerror(errno));
 
 #ifndef WIN32
 	/* add shebang header */
@@ -486,8 +486,8 @@ create_script_for_cluster_analyze(char **analyze_script_file_name)
 
 #ifndef WIN32
 	if (chmod(*analyze_script_file_name, S_IRWXU) != 0)
-		pg_fatal("Could not add execute permission to file \"%s\": %s\n",
-				 *analyze_script_file_name, getErrorText());
+		pg_fatal("could not add execute permission to file \"%s\": %s\n",
+				 *analyze_script_file_name, strerror(errno));
 #endif
 
 	termPQExpBuffer(&user_specification);
@@ -559,8 +559,8 @@ create_script_for_old_cluster_deletion(char **deletion_script_file_name)
 	prep_status("Creating script to delete old cluster");
 
 	if ((script = fopen_priv(*deletion_script_file_name, "w")) == NULL)
-		pg_fatal("Could not open file \"%s\": %s\n",
-				 *deletion_script_file_name, getErrorText());
+		pg_fatal("could not open file \"%s\": %s\n",
+				 *deletion_script_file_name, strerror(errno));
 
 #ifndef WIN32
 	/* add shebang header */
@@ -615,8 +615,8 @@ create_script_for_old_cluster_deletion(char **deletion_script_file_name)
 
 #ifndef WIN32
 	if (chmod(*deletion_script_file_name, S_IRWXU) != 0)
-		pg_fatal("Could not add execute permission to file \"%s\": %s\n",
-				 *deletion_script_file_name, getErrorText());
+		pg_fatal("could not add execute permission to file \"%s\": %s\n",
+				 *deletion_script_file_name, strerror(errno));
 #endif
 
 	check_ok();
@@ -819,8 +819,8 @@ check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster)
 		{
 			found = true;
 			if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-				pg_fatal("Could not open file \"%s\": %s\n",
-						 output_path, getErrorText());
+				pg_fatal("could not open file \"%s\": %s\n",
+						 output_path, strerror(errno));
 			if (!db_used)
 			{
 				fprintf(script, "Database: %s\n", active_db->db_name);
@@ -922,8 +922,8 @@ check_for_reg_data_type_usage(ClusterInfo *cluster)
 		{
 			found = true;
 			if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-				pg_fatal("Could not open file \"%s\": %s\n",
-						 output_path, getErrorText());
+				pg_fatal("could not open file \"%s\": %s\n",
+						 output_path, strerror(errno));
 			if (!db_used)
 			{
 				fprintf(script, "Database: %s\n", active_db->db_name);
@@ -1013,8 +1013,8 @@ check_for_jsonb_9_4_usage(ClusterInfo *cluster)
 		{
 			found = true;
 			if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-				pg_fatal("Could not open file \"%s\": %s\n",
-						 output_path, getErrorText());
+				pg_fatal("could not open file \"%s\": %s\n",
+						 output_path, strerror(errno));
 			if (!db_used)
 			{
 				fprintf(script, "Database: %s\n", active_db->db_name);
@@ -1089,8 +1089,8 @@ get_bin_version(ClusterInfo *cluster)
 
 	if ((output = popen(cmd, "r")) == NULL ||
 		fgets(cmd_output, sizeof(cmd_output), output) == NULL)
-		pg_fatal("Could not get pg_ctl version data using %s: %s\n",
-				 cmd, getErrorText());
+		pg_fatal("could not get pg_ctl version data using %s: %s\n",
+				 cmd, strerror(errno));
 
 	pclose(output);
 
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index d89cf196ab3..709f149a9e6 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -119,8 +119,8 @@ get_control_data(ClusterInfo *cluster, bool live_check)
 	fflush(stderr);
 
 	if ((output = popen(cmd, "r")) == NULL)
-		pg_fatal("Could not get control data using %s: %s\n",
-				 cmd, getErrorText());
+		pg_fatal("could not get control data using %s: %s\n",
+				 cmd, strerror(errno));
 
 	/* Only in <= 9.2 */
 	if (GET_MAJOR_VERSION(cluster->major_version) <= 902)
diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c
index dd309524414..6d04e5671db 100644
--- a/src/bin/pg_upgrade/exec.c
+++ b/src/bin/pg_upgrade/exec.c
@@ -191,7 +191,7 @@ pid_lock_file_exists(const char *datadir)
 		/* ENOTDIR means we will throw a more useful error later */
 		if (errno != ENOENT && errno != ENOTDIR)
 			pg_fatal("could not open file \"%s\" for reading: %s\n",
-					 path, getErrorText());
+					 path, strerror(errno));
 
 		return false;
 	}
@@ -285,7 +285,7 @@ check_data_dir(const char *pg_data)
 
 		if (stat(subDirName, &statBuf) != 0)
 			report_status(PG_FATAL, "check for \"%s\" failed: %s\n",
-						  subDirName, getErrorText());
+						  subDirName, strerror(errno));
 		else if (!S_ISDIR(statBuf.st_mode))
 			report_status(PG_FATAL, "%s is not a directory\n",
 						  subDirName);
@@ -309,7 +309,7 @@ check_bin_dir(ClusterInfo *cluster)
 	/* check bindir */
 	if (stat(cluster->bindir, &statBuf) != 0)
 		report_status(PG_FATAL, "check for \"%s\" failed: %s\n",
-					  cluster->bindir, getErrorText());
+					  cluster->bindir, strerror(errno));
 	else if (!S_ISDIR(statBuf.st_mode))
 		report_status(PG_FATAL, "%s is not a directory\n",
 					  cluster->bindir);
@@ -352,7 +352,7 @@ validate_exec(const char *dir, const char *cmdName)
 	 */
 	if (stat(path, &buf) < 0)
 		pg_fatal("check for \"%s\" failed: %s\n",
-				 path, getErrorText());
+				 path, strerror(errno));
 	else if (!S_ISREG(buf.st_mode))
 		pg_fatal("check for \"%s\" failed: not an executable file\n",
 				 path);
diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c
index 3e04c1a33e8..34619279928 100644
--- a/src/bin/pg_upgrade/file.c
+++ b/src/bin/pg_upgrade/file.c
@@ -19,9 +19,7 @@
 #include <fcntl.h>
 
 
-#ifndef WIN32
-static int	copy_file(const char *fromfile, const char *tofile);
-#else
+#ifdef WIN32
 static int	win32_pghardlink(const char *src, const char *dst);
 #endif
 
@@ -29,73 +27,29 @@ static int	win32_pghardlink(const char *src, const char *dst);
 /*
  * copyFile()
  *
- *	Copies a relation file from src to dst.
+ * Copies a relation file from src to dst.
+ * schemaName/relName are relation's SQL name (used for error messages only).
  */
-const char *
-copyFile(const char *src, const char *dst)
+void
+copyFile(const char *src, const char *dst,
+		 const char *schemaName, const char *relName)
 {
 #ifndef WIN32
-	if (copy_file(src, dst) == -1)
-#else
-	if (CopyFile(src, dst, true) == 0)
-#endif
-		return getErrorText();
-	else
-		return NULL;
-}
-
-
-/*
- * linkFile()
- *
- * Creates a hard link between the given relation files. We use
- * this function to perform a true in-place update. If the on-disk
- * format of the new cluster is bit-for-bit compatible with the on-disk
- * format of the old cluster, we can simply link each relation
- * instead of copying the data from the old cluster to the new cluster.
- */
-const char *
-linkFile(const char *src, const char *dst)
-{
-	if (pg_link_file(src, dst) == -1)
-		return getErrorText();
-	else
-		return NULL;
-}
-
-
-#ifndef WIN32
-static int
-copy_file(const char *srcfile, const char *dstfile)
-{
-#define COPY_BUF_SIZE (50 * BLCKSZ)
-
 	int			src_fd;
 	int			dest_fd;
 	char	   *buffer;
-	int			ret = 0;
-	int			save_errno = 0;
-
-	if ((srcfile == NULL) || (dstfile == NULL))
-	{
-		errno = EINVAL;
-		return -1;
-	}
 
-	if ((src_fd = open(srcfile, O_RDONLY | PG_BINARY, 0)) < 0)
-		return -1;
+	if ((src_fd = open(src, O_RDONLY | PG_BINARY, 0)) < 0)
+		pg_fatal("error while copying relation \"%s.%s\": could not open file \"%s\": %s\n",
+				 schemaName, relName, src, strerror(errno));
 
-	if ((dest_fd = open(dstfile, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
+	if ((dest_fd = open(dst, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
 						S_IRUSR | S_IWUSR)) < 0)
-	{
-		save_errno = errno;
+		pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %s\n",
+				 schemaName, relName, dst, strerror(errno));
 
-		if (src_fd != 0)
-			close(src_fd);
-
-		errno = save_errno;
-		return -1;
-	}
+	/* copy in fairly large chunks for best efficiency */
+#define COPY_BUF_SIZE (50 * BLCKSZ)
 
 	buffer = (char *) pg_malloc(COPY_BUF_SIZE);
 
@@ -105,47 +59,62 @@ copy_file(const char *srcfile, const char *dstfile)
 		ssize_t		nbytes = read(src_fd, buffer, COPY_BUF_SIZE);
 
 		if (nbytes < 0)
-		{
-			save_errno = errno;
-			ret = -1;
-			break;
-		}
+			pg_fatal("error while copying relation \"%s.%s\": could not read file \"%s\": %s\n",
+					 schemaName, relName, src, strerror(errno));
 
 		if (nbytes == 0)
 			break;
 
 		errno = 0;
-
 		if (write(dest_fd, buffer, nbytes) != nbytes)
 		{
 			/* if write didn't set errno, assume problem is no disk space */
 			if (errno == 0)
 				errno = ENOSPC;
-			save_errno = errno;
-			ret = -1;
-			break;
+			pg_fatal("error while copying relation \"%s.%s\": could not write file \"%s\": %s\n",
+					 schemaName, relName, dst, strerror(errno));
 		}
 	}
 
 	pg_free(buffer);
+	close(src_fd);
+	close(dest_fd);
 
-	if (src_fd != 0)
-		close(src_fd);
+#else							/* WIN32 */
+
+	if (CopyFile(src, dst, true) == 0)
+	{
+		_dosmaperr(GetLastError());
+		pg_fatal("error while copying relation \"%s.%s\" (\"%s\" to \"%s\"): %s\n",
+				 schemaName, relName, src, dst, strerror(errno));
+	}
 
-	if (dest_fd != 0)
-		close(dest_fd);
+#endif   /* WIN32 */
+}
 
-	if (save_errno != 0)
-		errno = save_errno;
 
-	return ret;
+/*
+ * linkFile()
+ *
+ * Hard-links a relation file from src to dst.
+ * schemaName/relName are relation's SQL name (used for error messages only).
+ */
+void
+linkFile(const char *src, const char *dst,
+		 const char *schemaName, const char *relName)
+{
+	if (pg_link_file(src, dst) < 0)
+		pg_fatal("error while creating link for relation \"%s.%s\" (\"%s\" to \"%s\"): %s\n",
+				 schemaName, relName, src, dst, strerror(errno));
 }
-#endif
 
 
 /*
  * rewriteVisibilityMap()
  *
+ * Transform a visibility map file, copying from src to dst.
+ * schemaName/relName are relation's SQL name (used for error messages only).
+ *
  * In versions of PostgreSQL prior to catversion 201603011, PostgreSQL's
  * visibility map included one bit per heap page; it now includes two.
  * When upgrading a cluster from before that time to a current PostgreSQL
@@ -156,8 +125,9 @@ copy_file(const char *srcfile, const char *dstfile)
  * remain set for the pages for which they were set previously.  The
  * all-frozen bits are never set by this conversion; we leave that to VACUUM.
  */
-const char *
-rewriteVisibilityMap(const char *fromfile, const char *tofile)
+void
+rewriteVisibilityMap(const char *fromfile, const char *tofile,
+					 const char *schemaName, const char *relName)
 {
 	int			src_fd;
 	int			dst_fd;
@@ -172,24 +142,18 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile)
 	/* Compute number of old-format bytes per new page */
 	rewriteVmBytesPerPage = (BLCKSZ - SizeOfPageHeaderData) / 2;
 
-	if ((fromfile == NULL) || (tofile == NULL))
-		return "Invalid old file or new file";
-
 	if ((src_fd = open(fromfile, O_RDONLY | PG_BINARY, 0)) < 0)
-		return getErrorText();
+		pg_fatal("error while copying relation \"%s.%s\": could not open file \"%s\": %s\n",
+				 schemaName, relName, fromfile, strerror(errno));
 
 	if (fstat(src_fd, &statbuf) != 0)
-	{
-		close(src_fd);
-		return getErrorText();
-	}
+		pg_fatal("error while copying relation \"%s.%s\": could not stat file \"%s\": %s\n",
+				 schemaName, relName, fromfile, strerror(errno));
 
 	if ((dst_fd = open(tofile, O_RDWR | O_CREAT | O_EXCL | PG_BINARY,
 					   S_IRUSR | S_IWUSR)) < 0)
-	{
-		close(src_fd);
-		return getErrorText();
-	}
+		pg_fatal("error while copying relation \"%s.%s\": could not create file \"%s\": %s\n",
+				 schemaName, relName, tofile, strerror(errno));
 
 	/* Save old file size */
 	src_filesize = statbuf.st_size;
@@ -218,9 +182,12 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile)
 
 		if ((bytesRead = read(src_fd, buffer, BLCKSZ)) != BLCKSZ)
 		{
-			close(dst_fd);
-			close(src_fd);
-			return getErrorText();
+			if (bytesRead < 0)
+				pg_fatal("error while copying relation \"%s.%s\": could not read file \"%s\": %s\n",
+						 schemaName, relName, fromfile, strerror(errno));
+			else
+				pg_fatal("error while copying relation \"%s.%s\": partial page found in file \"%s\"\n",
+						 schemaName, relName, fromfile);
 		}
 
 		totalBytesRead += BLCKSZ;
@@ -288,11 +255,14 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile)
 				((PageHeader) new_vmbuf)->pd_checksum =
 					pg_checksum_page(new_vmbuf, new_blkno);
 
+			errno = 0;
 			if (write(dst_fd, new_vmbuf, BLCKSZ) != BLCKSZ)
 			{
-				close(dst_fd);
-				close(src_fd);
-				return getErrorText();
+				/* if write didn't set errno, assume problem is no disk space */
+				if (errno == 0)
+					errno = ENOSPC;
+				pg_fatal("error while copying relation \"%s.%s\": could not write file \"%s\": %s\n",
+						 schemaName, relName, tofile, strerror(errno));
 			}
 
 			/* Advance for next new page */
@@ -306,8 +276,6 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile)
 	pg_free(new_vmbuf);
 	close(dst_fd);
 	close(src_fd);
-
-	return NULL;
 }
 
 void
@@ -320,16 +288,16 @@ check_hard_link(void)
 	snprintf(new_link_file, sizeof(new_link_file), "%s/PG_VERSION.linktest", new_cluster.pgdata);
 	unlink(new_link_file);		/* might fail */
 
-	if (pg_link_file(existing_file, new_link_file) == -1)
-	{
-		pg_fatal("Could not create hard link between old and new data directories: %s\n"
+	if (pg_link_file(existing_file, new_link_file) < 0)
+		pg_fatal("could not create hard link between old and new data directories: %s\n"
 				 "In link mode the old and new data directories must be on the same file system volume.\n",
-				 getErrorText());
-	}
+				 strerror(errno));
+
 	unlink(new_link_file);
 }
 
 #ifdef WIN32
+/* implementation of pg_link_file() on Windows */
 static int
 win32_pghardlink(const char *src, const char *dst)
 {
@@ -338,7 +306,10 @@ win32_pghardlink(const char *src, const char *dst)
 	 * http://msdn.microsoft.com/en-us/library/aa363860(VS.85).aspx
 	 */
 	if (CreateHardLinkA(dst, src, NULL) == 0)
+	{
+		_dosmaperr(GetLastError());
 		return -1;
+	}
 	else
 		return 0;
 }
@@ -353,7 +324,8 @@ fopen_priv(const char *path, const char *mode)
 	FILE	   *fp;
 
 	fp = fopen(path, mode);
-	umask(old_umask);
+
+	umask(old_umask);			/* we assume this can't change errno */
 
 	return fp;
 }
diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.c
index eaae976c536..b432b544b21 100644
--- a/src/bin/pg_upgrade/function.c
+++ b/src/bin/pg_upgrade/function.c
@@ -213,9 +213,9 @@ check_loadable_libraries(void)
 			found = true;
 
 			if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-				pg_fatal("Could not open file \"%s\": %s\n",
-						 output_path, getErrorText());
-			fprintf(script, "Could not load library \"%s\"\n%s\n",
+				pg_fatal("could not open file \"%s\": %s\n",
+						 output_path, strerror(errno));
+			fprintf(script, "could not load library \"%s\":\n%s\n",
 					lib,
 					PQerrorMessage(conn));
 		}
diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 45973554611..2e9a40c2b65 100644
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -422,8 +422,8 @@ adjust_data_dir(ClusterInfo *cluster)
 
 	if ((output = popen(cmd, "r")) == NULL ||
 		fgets(cmd_output, sizeof(cmd_output), output) == NULL)
-		pg_fatal("Could not get data directory using %s: %s\n",
-				 cmd, getErrorText());
+		pg_fatal("could not get data directory using %s: %s\n",
+				 cmd, strerror(errno));
 
 	pclose(output);
 
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index fa118e9e4fd..90c07205bf8 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -225,7 +225,7 @@ setup(char *argv0, bool *live_check)
 
 	/* get path to pg_upgrade executable */
 	if (find_my_exec(argv0, exec_path) < 0)
-		pg_fatal("Could not get path name to pg_upgrade: %s\n", getErrorText());
+		pg_fatal("%s: could not find own program executable\n", argv0);
 
 	/* Trim off program name and keep just path */
 	*last_dir_separator(exec_path) = '\0';
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 20f9a91087f..19dca83386f 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -367,10 +367,12 @@ bool		pid_lock_file_exists(const char *datadir);
 
 /* file.c */
 
-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);
-
+void copyFile(const char *src, const char *dst,
+		 const char *schemaName, const char *relName);
+void linkFile(const char *src, const char *dst,
+		 const char *schemaName, const char *relName);
+void rewriteVisibilityMap(const char *fromfile, const char *tofile,
+					 const char *schemaName, const char *relName);
 void		check_hard_link(void);
 FILE	   *fopen_priv(const char *path, const char *mode);
 
@@ -431,7 +433,6 @@ void		pg_fatal(const char *fmt,...) pg_attribute_printf(1, 2) pg_attribute_noret
 void		end_progress_output(void);
 void		prep_status(const char *fmt,...) pg_attribute_printf(1, 2);
 void		check_ok(void);
-const char *getErrorText(void);
 unsigned int str2uint(const char *str);
 void		pg_putenv(const char *var, const char *val);
 
diff --git a/src/bin/pg_upgrade/relfilenode.c b/src/bin/pg_upgrade/relfilenode.c
index 85cb717f748..c8c2a28f4e1 100644
--- a/src/bin/pg_upgrade/relfilenode.c
+++ b/src/bin/pg_upgrade/relfilenode.c
@@ -183,7 +183,6 @@ transfer_single_new_db(FileNameMap *maps, int size, char *old_tablespace)
 static void
 transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_frozenbit)
 {
-	const char *msg;
 	char		old_file[MAXPGPATH];
 	char		new_file[MAXPGPATH];
 	int			segno;
@@ -229,7 +228,7 @@ transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_fro
 				else
 					pg_fatal("error while checking for file existence \"%s.%s\" (\"%s\" to \"%s\"): %s\n",
 							 map->nspname, map->relname, old_file, new_file,
-							 getErrorText());
+							 strerror(errno));
 			}
 
 			/* If file is empty, just return */
@@ -242,35 +241,24 @@ transfer_relfile(FileNameMap *map, const char *type_suffix, bool vm_must_add_fro
 		/* Copying files might take some time, so give feedback. */
 		pg_log(PG_STATUS, "%s", old_file);
 
-		if (user_opts.transfer_mode == TRANSFER_MODE_COPY)
+		if (vm_must_add_frozenbit && strcmp(type_suffix, "_vm") == 0)
 		{
-			pg_log(PG_VERBOSE, "copying \"%s\" to \"%s\"\n", old_file, new_file);
-
-			/* Rewrite visibility map if needed */
-			if (vm_must_add_frozenbit && (strcmp(type_suffix, "_vm") == 0))
-				msg = rewriteVisibilityMap(old_file, new_file);
-			else
-				msg = copyFile(old_file, new_file);
-
-			if (msg)
-				pg_fatal("error while copying relation \"%s.%s\" (\"%s\" to \"%s\"): %s\n",
-						 map->nspname, map->relname, old_file, new_file, msg);
+			/* Need to rewrite visibility map format */
+			pg_log(PG_VERBOSE, "rewriting \"%s\" to \"%s\"\n",
+				   old_file, new_file);
+			rewriteVisibilityMap(old_file, new_file, map->nspname, map->relname);
+		}
+		else if (user_opts.transfer_mode == TRANSFER_MODE_COPY)
+		{
+			pg_log(PG_VERBOSE, "copying \"%s\" to \"%s\"\n",
+				   old_file, new_file);
+			copyFile(old_file, new_file, map->nspname, map->relname);
 		}
 		else
 		{
-			pg_log(PG_VERBOSE, "linking \"%s\" to \"%s\"\n", old_file, new_file);
-
-			/* Rewrite visibility map if needed */
-			if (vm_must_add_frozenbit && (strcmp(type_suffix, "_vm") == 0))
-				msg = rewriteVisibilityMap(old_file, new_file);
-			else
-				msg = linkFile(old_file, new_file);
-
-			if (msg)
-				pg_fatal("error while creating link for relation \"%s.%s\" (\"%s\" to \"%s\"): %s\n",
-						 map->nspname, map->relname, old_file, new_file, msg);
+			pg_log(PG_VERBOSE, "linking \"%s\" to \"%s\"\n",
+				   old_file, new_file);
+			linkFile(old_file, new_file, map->nspname, map->relname);
 		}
 	}
-
-	return;
 }
diff --git a/src/bin/pg_upgrade/tablespace.c b/src/bin/pg_upgrade/tablespace.c
index dfbce59ca30..8e09ec37293 100644
--- a/src/bin/pg_upgrade/tablespace.c
+++ b/src/bin/pg_upgrade/tablespace.c
@@ -90,8 +90,8 @@ get_tablespace_paths(void)
 							  os_info.old_tablespaces[tblnum]);
 			else
 				report_status(PG_FATAL,
-						   "cannot stat() tablespace directory \"%s\": %s\n",
-							os_info.old_tablespaces[tblnum], getErrorText());
+						  "could not stat tablespace directory \"%s\": %s\n",
+						   os_info.old_tablespaces[tblnum], strerror(errno));
 		}
 		if (!S_ISDIR(statBuf.st_mode))
 			report_status(PG_FATAL,
diff --git a/src/bin/pg_upgrade/util.c b/src/bin/pg_upgrade/util.c
index 5b24ec09174..aadc1cdd9df 100644
--- a/src/bin/pg_upgrade/util.c
+++ b/src/bin/pg_upgrade/util.c
@@ -232,21 +232,6 @@ get_user_info(char **user_name_p)
 }
 
 
-/*
- * getErrorText()
- *
- *	Returns the text of the most recent error
- */
-const char *
-getErrorText(void)
-{
-#ifdef WIN32
-	_dosmaperr(GetLastError());
-#endif
-	return pg_strdup(strerror(errno));
-}
-
-
 /*
  *	str2uint()
  *
diff --git a/src/bin/pg_upgrade/version.c b/src/bin/pg_upgrade/version.c
index 36c299c1016..3c7c5facf48 100644
--- a/src/bin/pg_upgrade/version.c
+++ b/src/bin/pg_upgrade/version.c
@@ -52,7 +52,8 @@ new_9_0_populate_pg_largeobject_metadata(ClusterInfo *cluster, bool check_mode)
 				PQExpBufferData connectbuf;
 
 				if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-					pg_fatal("could not open file \"%s\": %s\n", output_path, getErrorText());
+					pg_fatal("could not open file \"%s\": %s\n", output_path,
+							 strerror(errno));
 
 				initPQExpBuffer(&connectbuf);
 				appendPsqlMetaConnect(&connectbuf, active_db->db_name);
@@ -150,7 +151,8 @@ old_9_3_check_for_line_data_type_usage(ClusterInfo *cluster)
 		{
 			found = true;
 			if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
-				pg_fatal("could not open file \"%s\": %s\n", output_path, getErrorText());
+				pg_fatal("could not open file \"%s\": %s\n", output_path,
+						 strerror(errno));
 			if (!db_used)
 			{
 				fprintf(script, "Database: %s\n", active_db->db_name);
-- 
GitLab