On Wed, Dec 26, 2018 at 09:08:23PM +0000, Tom Lane wrote:
> 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.
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);
+ }
Wouldn't it be more simple to remove stat() and just call
BasicOpenFilePerm, complaining with FATAL about any failures,
including EACCES, on the way? The code is racy as designed, even if
that's not a big deal for recovery purposes.
--
Michael