From 8528e3d849a896f8711c56fb41eae56f8c986729 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 26 Dec 2018 16:08:17 -0500 Subject: [PATCH] Fix failure to check for open() or fsync() failures. While it seems OK to not be concerned about fsync() failure for a pre-existing signal file, it's not OK to not even check for open() failure. This at least causes complaints from static analyzers, and I think on some platforms passing -1 to fsync() or close() might trigger assertion-type failures. Also add (void) casts to make clear that we're ignoring fsync's result intentionally. Oversights in commit 2dedf4d9a, noted by Coverity. --- src/backend/access/transam/xlog.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index c80b14ed97..5729867879 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -5327,7 +5327,8 @@ readRecoverySignalFile(void) /* * Check for recovery signal files and if found, fsync them since they - * represent server state information. + * represent server state information. We don't sweat too much about the + * possibility of fsync failure, however. * * If present, standby signal file takes precedence. If neither is present * then we won't enter archive recovery. @@ -5338,8 +5339,11 @@ readRecoverySignalFile(void) fd = BasicOpenFilePerm(STANDBY_SIGNAL_FILE, O_RDWR | PG_BINARY | get_sync_bit(sync_method), S_IRUSR | S_IWUSR); - pg_fsync(fd); - close(fd); + if (fd >= 0) + { + (void) pg_fsync(fd); + close(fd); + } standby_signal_file_found = true; } else if (stat(RECOVERY_SIGNAL_FILE, &stat_buf) == 0) @@ -5348,8 +5352,11 @@ readRecoverySignalFile(void) fd = BasicOpenFilePerm(RECOVERY_SIGNAL_FILE, O_RDWR | PG_BINARY | get_sync_bit(sync_method), S_IRUSR | S_IWUSR); - pg_fsync(fd); - close(fd); + if (fd >= 0) + { + (void) pg_fsync(fd); + close(fd); + } recovery_signal_file_found = true; }