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.

diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index f41513517274363397689ce1a17845d8a060b77d..8511e57cf7df2390fc63dbcef286bab40e2d91db 100644 (file)
--- 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 d9ad596bf0603d887686e1413d95e627dab8718c..d4de8ddcf783db4999bca1337d5886b84bf6f559 100644 (file)
--- 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 8d679dab615765ed0413f08e861a2a8cd7aa8ed3..35a280613f20f0627bf9137f8322bb3ed2727018 100644 (file)
--- 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);
 };
 
 /*