Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 2 commits into from Jun 2, 2016
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c2f28e9
WT-2672 handle system calls that don't set errno
keithbostic Jun 1, 2016
49bed27
POSIX requires posix_madvise return an errno value, but some OS versions
keithbostic Jun 1, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions dist/s_string.ok
View file
Open in desktop
Expand Up @@ -442,6 +442,7 @@ bzDecompressInit
bzalloc
bzfree
bzip
call's
calloc
cas
catfmt
Expand Down
27 changes: 19 additions & 8 deletions src/include/os.h
View file
Open in desktop
Expand Up @@ -9,15 +9,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: \
Expand Down
2 changes: 1 addition & 1 deletion src/os_posix/os_dir.c
View file
Open in desktop
Expand Up @@ -38,7 +38,7 @@ __wt_posix_directory_list(WT_FILE_SYSTEM *file_system,
dirallocsz = 0;
entries = NULL;

WT_SYSCALL_RETRY(((dirp = opendir(directory)) == NULL ? 1 : 0), ret);
WT_SYSCALL_RETRY(((dirp = opendir(directory)) == NULL ? -1 : 0), ret);
if (ret != 0)
WT_RET_MSG(session, ret,
"%s: directory-list: opendir", directory);
Expand Down
30 changes: 22 additions & 8 deletions src/os_posix/os_fs.c
View file
Open in desktop
Expand Up @@ -53,7 +53,7 @@ __posix_sync(
* "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);
/*
Expand Down Expand Up @@ -92,7 +92,7 @@ __posix_directory_sync(
session = (WT_SESSION_IMPL *)wt_session;

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_RET_MSG(session, ret, "%s: directory-sync: open", path);

Expand Down Expand Up @@ -151,10 +151,17 @@ __posix_fs_remove(

session = (WT_SESSION_IMPL *)wt_session;

WT_SYSCALL_RETRY(remove(name), ret);
/*
* 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(name), ret);
if (ret == 0)
return (0);
WT_RET_MSG(session, ret, "%s: file-remove: remove", name);
WT_RET_MSG(session, ret, "%s: file-remove: unlink", name);
}

/*
Expand All @@ -172,7 +179,14 @@ __posix_fs_rename(WT_FILE_SYSTEM *file_system,

session = (WT_SESSION_IMPL *)wt_session;

WT_SYSCALL_RETRY(rename(from, to), ret);
/*
* 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, to) != 0 ? -1 : 0, ret);
if (ret == 0)
return (0);
WT_RET_MSG(session, ret, "%s to %s: file-rename: rename", from, to);
Expand Down Expand Up @@ -295,7 +309,7 @@ __posix_file_lock(
fl.l_type = lock ? F_WRLCK : F_UNLCK;
fl.l_whence = SEEK_SET;

WT_SYSCALL_RETRY(fcntl(pfh->fd, F_SETLK, &fl), ret);
WT_SYSCALL_RETRY(fcntl(pfh->fd, F_SETLK, &fl) == -1 ? -1 : 0, ret);
if (ret == 0)
return (0);
WT_RET_MSG(session, ret, "%s: handle-lock: fcntl", file_handle->name);
Expand Down Expand Up @@ -533,7 +547,7 @@ __posix_open_file(WT_FILE_SYSTEM *file_system, WT_SESSION *wt_session,
f |= O_CLOEXEC;
#endif
WT_SYSCALL_RETRY((
(pfh->fd = open(name, f, 0444)) == -1 ? 1 : 0), ret);
(pfh->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_open_file_cloexec(session, pfh->fd, name));
Expand Down Expand Up @@ -587,7 +601,7 @@ __posix_open_file(WT_FILE_SYSTEM *file_system, WT_SESSION *wt_session,
#endif
}

WT_SYSCALL_RETRY(((pfh->fd = open(name, f, mode)) == -1 ? 1 : 0), ret);
WT_SYSCALL_RETRY(((pfh->fd = open(name, f, mode)) == -1 ? -1 : 0), ret);
if (ret != 0)
WT_ERR_MSG(session, ret,
pfh->direct_io ?
Expand Down
11 changes: 8 additions & 3 deletions src/os_posix/os_map.c
View file
Open in desktop
Expand Up @@ -102,10 +102,13 @@ __wt_posix_map_preload(WT_FILE_HANDLE *fh,
* size, so we will be conservative.
*/
length &= ~(size_t)(conn->page_size - 1);
if (length <= (size_t)conn->page_size)
return (0);

if (length <= (size_t)conn->page_size ||
(ret = posix_madvise(blk, length, POSIX_MADV_WILLNEED)) == 0)
WT_SYSCALL_RETRY(posix_madvise(blk, length, POSIX_MADV_WILLNEED), ret);
if (ret == 0)
return (0);

WT_RET_MSG(session, ret,
"%s: memory-map preload: posix_madvise: POSIX_MADV_WILLNEED",
fh->name);
Expand Down Expand Up @@ -135,8 +138,10 @@ __wt_posix_map_discard(WT_FILE_HANDLE *fh,
blk = (void *)((uintptr_t)map & ~(uintptr_t)(conn->page_size - 1));
length += WT_PTRDIFF(map, blk);

if ((ret = posix_madvise(blk, length, POSIX_MADV_DONTNEED)) == 0)
WT_SYSCALL_RETRY(posix_madvise(blk, length, POSIX_MADV_DONTNEED), ret);
if (ret == 0)
return (0);

WT_RET_MSG(session, ret,
"%s: memory-map discard: posix_madvise: POSIX_MADV_DONTNEED",
fh->name);
Expand Down
5 changes: 0 additions & 5 deletions src/os_win/os_fs.c
View file
Open in desktop
Expand Up @@ -169,11 +169,6 @@ __win_file_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
*
Expand Down