From b935eb7da37254a18c48acc7b07517c8dfbb2339 Mon Sep 17 00:00:00 2001
From: Magnus Hagander <magnus@hagander.net>
Date: Wed, 12 Apr 2017 13:43:59 +0200
Subject: [PATCH] Fix reversed check of return value from sync

While at it also update the comments in walmethods.h to make it less
likely for mistakes like this to appear in the future (thanks to Tom for
improvements to the comments).

And finally, in passing change the return type of walmethod.getlasterror
to being const, also per suggestion from Tom.
---
 src/bin/pg_basebackup/receivelog.c |  5 +++-
 src/bin/pg_basebackup/walmethods.c |  4 +--
 src/bin/pg_basebackup/walmethods.h | 46 +++++++++++++++++++++++++++++-
 3 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index f415135172..8511e57cf7 100644
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -132,8 +132,11 @@ open_walfile(StreamCtl *stream, XLogRecPtr startpoint)
 			}
 
 			/* fsync file in case of a previous crash */
-			if (!stream->walmethod->sync(f))
+			if (stream->walmethod->sync(f) != 0)
 			{
+				fprintf(stderr,
+						_("%s: could not sync existing transaction log file \"%s\": %s\n"),
+						progname, fn, stream->walmethod->getlasterror());
 				stream->walmethod->close(f, CLOSE_UNLINK);
 				return false;
 			}
diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
index d9ad596bf0..d4de8ddcf7 100644
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -61,7 +61,7 @@ typedef struct DirectoryMethodFile
 #endif
 }	DirectoryMethodFile;
 
-static char *
+static const char *
 dir_getlasterror(void)
 {
 	/* Directory method always sets errno, so just use strerror */
@@ -406,7 +406,7 @@ static TarMethodData *tar_data = NULL;
 #define tar_clear_error() tar_data->lasterror[0] = '\0'
 #define tar_set_error(msg) strlcpy(tar_data->lasterror, msg, sizeof(tar_data->lasterror))
 
-static char *
+static const char *
 tar_getlasterror(void)
 {
 	/*
diff --git a/src/bin/pg_basebackup/walmethods.h b/src/bin/pg_basebackup/walmethods.h
index 8d679dab61..35a280613f 100644
--- a/src/bin/pg_basebackup/walmethods.h
+++ b/src/bin/pg_basebackup/walmethods.h
@@ -19,19 +19,63 @@ typedef enum
 	CLOSE_NO_RENAME
 }	WalCloseMethod;
 
+/*
+ * A WalWriteMethod structure represents the different methods used
+ * to write the streaming WAL as it's received.
+ *
+ * All methods that have a failure return indicator will set state
+ * allowing the getlasterror() method to return a suitable message.
+ * Commonly, errno is this state (or part of it); so callers must take
+ * care not to clobber errno between a failed method call and use of
+ * getlasterror() to retrieve the message.
+ */
 typedef struct WalWriteMethod WalWriteMethod;
 struct WalWriteMethod
 {
+	/*
+	 * Open a target file. Returns Walfile, or NULL if open failed. If a temp
+	 * suffix is specified, a file with that name will be opened, and then
+	 * automatically renamed in close(). If pad_to_size is specified, the file
+	 * will be padded with NUL up to that size, if supported by the Walmethod.
+	 */
 	Walfile(*open_for_write) (const char *pathname, const char *temp_suffix, size_t pad_to_size);
+
+	/*
+	 * Close an open Walfile, using one or more methods for handling automatic
+	 * unlinking etc. Returns 0 on success, other values for error.
+	 */
 	int			(*close) (Walfile f, WalCloseMethod method);
+
+	/* Check if a file exist */
 	bool		(*existsfile) (const char *pathname);
+
+	/* Return the size of a file, or -1 on failure. */
 	ssize_t		(*get_file_size) (const char *pathname);
 
+	/*
+	 * Write count number of bytes to the file, and return the number of bytes
+	 * actually written or -1 for error.
+	 */
 	ssize_t		(*write) (Walfile f, const void *buf, size_t count);
+
+	/* Return the current position in a file or -1 on error */
 	off_t		(*get_current_pos) (Walfile f);
+
+	/*
+	 * fsync the contents of the specified file. Returns 0 on success.
+	 */
 	int			(*sync) (Walfile f);
+
+	/*
+	 * Clean up the Walmethod, closing any shared resources. For methods like
+	 * tar, this includes writing updated headers. Returns true if the
+	 * close/write/sync of shared resources succeeded, otherwise returns false
+	 * (but the resources are still closed).
+	 */
 	bool		(*finish) (void);
-	char	   *(*getlasterror) (void);
+
+	/* Return a text for the last error in this Walfile */
+	const char *(*getlasterror) (void);
 };
 
 /*