WT-2672 handle system calls that don't set errno (#2765)

Define system call success as a 0 return, and split error handling into two parts: if the call returns -1, use errno, otherwise expect the failing return to be an error value.

Replace calls to remove with unlink, so we know errno will be set.  Do the best we can with rename, there's no easy workaround.

POSIX requires posix_madvise return an errno value, but some OS versions return a -1/errno pair instead (at least FreeBSD and OS X). I don't care about retrying posix_madvise calls on failure, but since WT_SYSCALL_RETRY includes the necessary error handling magic, wrap the posix_madvise calls in WT_SYSCALL_RETRY.

(cherry picked from commit ced588a)

dist/s_string.ok

@@ -439,6 +439,7 @@ bzDecompressInit
 bzalloc
 bzfree
 bzip
+call's
 calloc
 cas
 catfmt

src/include/os.h

@@ -17,15 +17,26 @@
 #define	WT_SYSCALL_RETRY(call, ret) do {				\
 	int __retry;							\
 	for (__retry = 0; __retry < 10; ++__retry) {			\
-		if ((call) == 0) {					\
-			(ret) = 0;					\
-			break;						\
-		}							\
-		switch ((ret) = __wt_errno()) {				\
-		case 0:							\
-			/* The call failed but didn't set errno. */	\
-			(ret) = WT_ERROR;				\
+		/*							\
+		 * A call returning 0 indicates success; any call where	\
+		 * 0 is not the only successful return must provide an	\
+		 * expression evaluating to 0 in all successful	cases.	\
+		 */							\
+		if (((ret) = (call)) == 0)				\
 			break;						\
+		/*							\
+		 * The call's error was either returned by the call or	\
+		 * is in errno, and there are cases where it depends on	\
+		 * the software release as to which it is (for example,	\
+		 * posix_fadvise on FreeBSD and OS X). Failing calls	\
+		 * must either return a non-zero error value, or -1 if	\
+		 * the error value is in errno. (The WiredTiger errno	\
+		 * function returns WT_ERROR if errno is 0, which isn't	\
+		 * ideal but won't discard the failure.)		\
+		 */							\
+		if ((ret) == -1)					\
+			(ret) = __wt_errno();				\
+		switch (ret) {						\
 		case EAGAIN:						\
 		case EBUSY:						\
 		case EINTR:						\

src/os_posix/os_dir.c

@@ -36,7 +36,7 @@ __wt_posix_directory_list(WT_SESSION_IMPL *session, const char *dir,
 	dirsz = 0;
 	entries = NULL;
 
-	WT_SYSCALL_RETRY(((dirp = opendir(path)) == NULL ? 1 : 0), ret);
+	WT_SYSCALL_RETRY(((dirp = opendir(path)) == NULL ? -1 : 0), ret);
 	if (ret != 0)
 		WT_ERR_MSG(session, ret, "%s: directory-list: opendir", path);
 

src/os_posix/os_fs.c

@@ -52,7 +52,7 @@ __posix_sync(WT_SESSION_IMPL *session,
 	 * "This is currently implemented on HFS, MS-DOS (FAT), and Universal
 	 * Disk Format (UDF) file systems."
 	 */
-	WT_SYSCALL_RETRY(fcntl(fd, F_FULLFSYNC, 0), ret);
+	WT_SYSCALL_RETRY(fcntl(fd, F_FULLFSYNC, 0) == -1 ? -1 : 0, ret);
 	if (ret == 0)
 		return (0);
 	/*
@@ -107,7 +107,7 @@ __posix_directory_sync(WT_SESSION_IMPL *session, const char *path)
 	}
 
 	WT_SYSCALL_RETRY((
-	    (fd = open(path, O_RDONLY, 0444)) == -1 ? 1 : 0), ret);
+	    (fd = open(path, O_RDONLY, 0444)) == -1 ? -1 : 0), ret);
 	if (ret != 0)
 		WT_ERR_MSG(session, ret, "%s: directory-sync: open", path);
 
@@ -172,14 +172,19 @@ __posix_file_remove(WT_SESSION_IMPL *session, const char *name)
 #endif
 
 	WT_RET(__wt_filename(session, name, &path));
-	name = path;
-
-	WT_SYSCALL_RETRY(remove(name), ret);
-	if (ret != 0)
-		__wt_err(session, ret, "%s: file-remove: remove", name);
 
+	/*
+	 * ISO C doesn't require remove return -1 on failure or set errno (note
+	 * POSIX 1003.1 extends C with those requirements). Regardless, use the
+	 * unlink system call, instead of remove, to simplify error handling;
+	 * where we're not doing any special checking for standards compliance,
+	 * using unlink may be marginally safer.
+	 */
+	WT_SYSCALL_RETRY(unlink(path), ret);
 	__wt_free(session, path);
-	return (ret);
+	if (ret == 0)
+		return (0);
+	WT_RET_MSG(session, ret, "%s: file-remove: unlink", name);
 }
 
 /*
@@ -203,18 +208,22 @@ __posix_file_rename(WT_SESSION_IMPL *session, const char *from, const char *to)
 
 	from_path = to_path = NULL;
 	WT_ERR(__wt_filename(session, from, &from_path));
-	from = from_path;
 	WT_ERR(__wt_filename(session, to, &to_path));
-	to = to_path;
 
-	WT_SYSCALL_RETRY(rename(from, to), ret);
-	if (ret != 0)
-		__wt_err(session, ret,
-		    "%s to %s: file-rename: rename", from, to);
+	/*
+	 * ISO C doesn't require rename return -1 on failure or set errno (note
+	 * POSIX 1003.1 extends C with those requirements). Be cautious, force
+	 * any non-zero return to -1 so we'll check errno. We can still end up
+	 * with the wrong errno (if errno is garbage), or the generic WT_ERROR
+	 * return (if errno is 0), but we've done the best we can.
+	 */
+	WT_SYSCALL_RETRY(rename(from_path, to_path) != 0 ? -1 : 0, ret);
 
 err:	__wt_free(session, from_path);
 	__wt_free(session, to_path);
-	return (ret);
+	if (ret == 0)
+		return (0);
+	WT_RET_MSG(session, ret, "%s to %s: file-rename: rename", from, to);
 }
 
 /*
@@ -360,7 +369,7 @@ __posix_handle_lock(WT_SESSION_IMPL *session, WT_FH *fh, bool lock)
 	fl.l_type = lock ? F_WRLCK : F_UNLCK;
 	fl.l_whence = SEEK_SET;
 
-	WT_SYSCALL_RETRY(fcntl(fh->fd, F_SETLK, &fl), ret);
+	WT_SYSCALL_RETRY(fcntl(fh->fd, F_SETLK, &fl) == -1 ? -1 : 0, ret);
 	if (ret == 0)
 		return (0);
 	WT_RET_MSG(session, ret, "%s: handle-lock: fcntl", fh->name);
@@ -560,7 +569,7 @@ __posix_handle_open(WT_SESSION_IMPL *session,
 		f |= O_CLOEXEC;
 #endif
 		WT_SYSCALL_RETRY((
-		    (fd = open(name, f, 0444)) == -1 ? 1 : 0), ret);
+		    (fd = open(name, f, 0444)) == -1 ? -1 : 0), ret);
 		if (ret != 0)
 			WT_ERR_MSG(session, ret, "%s: handle-open: open", name);
 		WT_ERR(__posix_handle_open_cloexec(session, fd, name));
@@ -622,7 +631,7 @@ __posix_handle_open(WT_SESSION_IMPL *session,
 #endif
 	}
 
-	WT_SYSCALL_RETRY(((fd = open(name, f, mode)) == -1 ? 1 : 0), ret);
+	WT_SYSCALL_RETRY(((fd = open(name, f, mode)) == -1 ? -1 : 0), ret);
 	if (ret != 0)
 		WT_ERR_MSG(session, ret,
 		    direct_io ?

src/os_posix/os_map.c

@@ -98,6 +98,7 @@ __posix_map_preload_madvise(
 	if (size <= (size_t)conn->page_size ||
 	    (ret = posix_madvise(blk, size, POSIX_MADV_WILLNEED)) == 0)
 		return (0);
+
 	WT_RET_MSG(session, ret,
 	    "%s: memory-map preload: posix_madvise: POSIX_MADV_WILLNEED",
 	    fh->name);
@@ -145,6 +146,7 @@ __posix_map_discard_madvise(
 
 	if ((ret = posix_madvise(blk, size, POSIX_MADV_DONTNEED)) == 0)
 		return (0);
+
 	WT_RET_MSG(session, ret,
 	    "%s: memory-map discard: posix_madvise: POSIX_MADV_DONTNEED",
 	    fh->name);

src/os_win/os_fs.c

@@ -286,11 +286,6 @@ __win_handle_lock(WT_SESSION_IMPL *session, WT_FH *fh, bool lock)
 	 * WiredTiger requires this function be able to acquire locks past
 	 * the end of file.
 	 *
-	 * Note we're using fcntl(2) locking: all fcntl locks associated with a
-	 * file for a given process are removed when any file descriptor for the
-	 * file is closed by the process, even if a lock was never requested for
-	 * that file descriptor.
-	 *
 	 * http://msdn.microsoft.com/
 	 *    en-us/library/windows/desktop/aa365202%28v=vs.85%29.aspx
 	 *