You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucy.apache.org by nw...@apache.org on 2017/02/20 15:24:49 UTC
lucy git commit: Make FSFileHandle always use Windows handles
Repository: lucy
Updated Branches:
refs/heads/master d14281228 -> 5f15a92bd
Make FSFileHandle always use Windows handles
Switch FSFileHandle over to WinAPI for open/close/write.
Add sanity checks to FileHandle_do_open.
Fix minor bug in Unix Write method (-1 return value was added to file
length). Make Windows Read method use synchronous API and add an extra
check for unexpected EOF.
Fixes LUCY-321.
Project: http://git-wip-us.apache.org/repos/asf/lucy/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucy/commit/5f15a92b
Tree: http://git-wip-us.apache.org/repos/asf/lucy/tree/5f15a92b
Diff: http://git-wip-us.apache.org/repos/asf/lucy/diff/5f15a92b
Branch: refs/heads/master
Commit: 5f15a92bde4c63e2b0d1c3d8b74a294ed8a3d1f0
Parents: d142812
Author: Nick Wellnhofer <we...@aevum.de>
Authored: Sun Feb 19 22:43:03 2017 +0100
Committer: Nick Wellnhofer <we...@aevum.de>
Committed: Mon Feb 20 16:10:18 2017 +0100
----------------------------------------------------------------------
core/Lucy/Store/FSFileHandle.c | 450 +++++++++++++--------------
core/Lucy/Store/FileHandle.c | 28 +-
core/Lucy/Store/RAMFileHandle.c | 6 +-
test/Lucy/Test/Store/MockFileHandle.c | 2 +-
test/Lucy/Test/Store/TestFSFileHandle.c | 20 +-
test/Lucy/Test/Store/TestFileHandle.c | 2 +-
6 files changed, 250 insertions(+), 258 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucy/blob/5f15a92b/core/Lucy/Store/FSFileHandle.c
----------------------------------------------------------------------
diff --git a/core/Lucy/Store/FSFileHandle.c b/core/Lucy/Store/FSFileHandle.c
index 494d661..fff9175 100644
--- a/core/Lucy/Store/FSFileHandle.c
+++ b/core/Lucy/Store/FSFileHandle.c
@@ -19,45 +19,20 @@
#include "charmony.h"
-#include <stdio.h>
-#include <fcntl.h> // open, POSIX flags
-
-#ifdef CHY_HAS_UNISTD_H
- #include <unistd.h> // close
-#endif
-
-#ifdef CHY_HAS_SYS_MMAN_H
- #include <sys/mman.h>
-#elif defined(CHY_HAS_WINDOWS_H)
- #include <windows.h>
- #include <io.h>
-#else
- #error "No support for memory mapped files"
-#endif
-
#include "Lucy/Store/ErrorMessage.h"
#include "Lucy/Store/FSFileHandle.h"
#include "Lucy/Store/FileWindow.h"
-// Convert FileHandle flags to POSIX flags.
-static CFISH_INLINE int
-SI_posix_flags(uint32_t fh_flags) {
- int posix_flags = 0;
- if (fh_flags & FH_WRITE_ONLY) { posix_flags |= O_WRONLY; }
- if (fh_flags & FH_READ_ONLY) { posix_flags |= O_RDONLY; }
- if (fh_flags & FH_CREATE) { posix_flags |= O_CREAT; }
- if (fh_flags & FH_EXCLUSIVE) { posix_flags |= O_EXCL; }
-#ifdef O_LARGEFILE
- posix_flags |= O_LARGEFILE;
-#endif
-#ifdef _O_BINARY
- posix_flags |= _O_BINARY;
-#endif
- return posix_flags;
-}
-
#define IS_64_BIT (CHY_SIZEOF_PTR == 8 ? 1 : 0)
+// Architecture- and OS- specific initialization.
+static bool
+S_init(FSFileHandleIVARS *ivars, String *path, uint32_t flags);
+
+// Close file in an OS-specific way.
+static bool
+S_close(FSFileHandleIVARS *ivars);
+
// Memory map a region of the file with shared (read-only) permissions. If
// the requested length is 0, return NULL. If an error occurs, return NULL
// and set the global error object.
@@ -74,16 +49,6 @@ static CFISH_INLINE bool
SI_window(FSFileHandle *self, FSFileHandleIVARS *ivars, FileWindow *window,
int64_t offset, int64_t len);
-// Architecture- and OS- specific initialization for a read-only FSFileHandle.
-static CFISH_INLINE bool
-SI_init_read_only(FSFileHandle *self, FSFileHandleIVARS *ivars);
-
-// Windows-specific routine needed for closing read-only handles.
-#ifdef CHY_HAS_WINDOWS_H
-static CFISH_INLINE bool
-SI_close_win_handles(FSFileHandle *self);
-#endif
-
FSFileHandle*
FSFH_open(String *path, uint32_t flags) {
FSFileHandle *self = (FSFileHandle*)Class_Make_Obj(FSFILEHANDLE);
@@ -92,71 +57,31 @@ FSFH_open(String *path, uint32_t flags) {
FSFileHandle*
FSFH_do_open(FSFileHandle *self, String *path, uint32_t flags) {
- FH_do_open((FileHandle*)self, path, flags);
+ if (!FH_do_open((FileHandle*)self, path, flags)) { return NULL; }
FSFileHandleIVARS *const ivars = FSFH_IVARS(self);
+
if (!path || !Str_Get_Size(path)) {
ErrMsg_set("Missing required param 'path'");
- CFISH_DECREF(self);
+ DECREF(self);
return NULL;
}
// Attempt to open file.
- if (flags & FH_WRITE_ONLY) {
- char *path_ptr = Str_To_Utf8(path);
- ivars->fd = open(path_ptr, SI_posix_flags(flags), 0666);
- FREEMEM(path_ptr);
- if (ivars->fd == -1) {
- ivars->fd = 0;
- ErrMsg_set_with_errno("Attempt to open '%o' failed", path);
- CFISH_DECREF(self);
- return NULL;
- }
- if (flags & FH_EXCLUSIVE) {
- ivars->len = 0;
- }
- else {
- // Derive length.
- ivars->len = chy_lseek64(ivars->fd, INT64_C(0), SEEK_END);
- if (ivars->len == -1) {
- ErrMsg_set_with_errno("lseek64 on %o failed", path);
- CFISH_DECREF(self);
- return NULL;
- }
- else {
- int64_t check_val
- = chy_lseek64(ivars->fd, INT64_C(0), SEEK_SET);
- if (check_val == -1) {
- ErrMsg_set_with_errno("lseek64 on %o failed", path);
- CFISH_DECREF(self);
- return NULL;
- }
- }
- }
+ if (!S_init(ivars, path, flags)) {
+ DECREF(self);
+ return NULL;
}
- else if (flags & FH_READ_ONLY) {
- if (SI_init_read_only(self, ivars)) {
- // On 64-bit systems, map the whole file up-front.
- if (IS_64_BIT && ivars->len) {
- ivars->buf = (char*)SI_map(self, ivars, 0, ivars->len);
- if (!ivars->buf) {
- // An error occurred during SI_map, which has set
- // the global error object for us already.
- CFISH_DECREF(self);
- return NULL;
- }
- }
- }
- else {
- CFISH_DECREF(self);
+
+ // On 64-bit systems, map the whole file up-front.
+ if (IS_64_BIT && (flags & FH_READ_ONLY) && ivars->len) {
+ ivars->buf = (char*)SI_map(self, ivars, 0, ivars->len);
+ if (!ivars->buf) {
+ // An error occurred during SI_map, which has set
+ // the global error object for us already.
+ DECREF(self);
return NULL;
}
}
- else {
- ErrMsg_set("Must specify FH_READ_ONLY or FH_WRITE_ONLY to open '%o'",
- path);
- CFISH_DECREF(self);
- return NULL;
- }
return self;
}
@@ -171,45 +96,7 @@ FSFH_Close_IMP(FSFileHandle *self) {
ivars->buf = NULL;
}
- // Close system-specific handles.
- if (ivars->fd) {
- if (close(ivars->fd)) {
- ErrMsg_set_with_errno("Failed to close file");
- return false;
- }
- ivars->fd = 0;
- }
- #if (defined(CHY_HAS_WINDOWS_H) && !defined(CHY_HAS_SYS_MMAN_H))
- if (ivars->win_fhandle) {
- if (!SI_close_win_handles(self)) { return false; }
- }
- #endif
-
- return true;
-}
-
-bool
-FSFH_Write_IMP(FSFileHandle *self, const void *data, size_t len) {
- FSFileHandleIVARS *const ivars = FSFH_IVARS(self);
-
- if (len) {
- // Write data, track file length, check for errors.
- int64_t check_val = write(ivars->fd, data, len);
- ivars->len += check_val;
- if ((size_t)check_val != len) {
- if (check_val == -1) {
- ErrMsg_set_with_errno("Error when writing %u64 bytes",
- (uint64_t)len);
- }
- else {
- ErrMsg_set("Attempted to write %u64 bytes, but wrote %i64",
- (uint64_t)len, check_val);
- }
- return false;
- }
- }
-
- return true;
+ return S_close(ivars);
}
int64_t
@@ -335,42 +222,95 @@ FSFH_Release_Window_IMP(FSFileHandle *self, FileWindow *window) {
#ifdef CHY_HAS_SYS_MMAN_H
-static CFISH_INLINE bool
-SI_init_read_only(FSFileHandle *self, FSFileHandleIVARS *ivars) {
- UNUSED_VAR(self);
+#include <fcntl.h> // open, POSIX flags
+#include <sys/mman.h>
+#include <unistd.h> // close
+
+static bool
+S_init(FSFileHandleIVARS *ivars, String *path, uint32_t flags) {
+ int posix_flags = 0;
+ if (flags & FH_WRITE_ONLY) { posix_flags |= O_WRONLY; }
+ if (flags & FH_READ_ONLY) { posix_flags |= O_RDONLY; }
+ if (flags & FH_CREATE) { posix_flags |= O_CREAT; }
+ if (flags & FH_EXCLUSIVE) { posix_flags |= O_EXCL; }
+#ifdef O_LARGEFILE
+ posix_flags |= O_LARGEFILE;
+#endif
- // Open.
- char *path_ptr = Str_To_Utf8(ivars->path);
- ivars->fd = open(path_ptr, SI_posix_flags(ivars->flags), 0666);
+ char *path_ptr = Str_To_Utf8(path);
+ int fd = open(path_ptr, posix_flags, 0666);
FREEMEM(path_ptr);
- if (ivars->fd == -1) {
- ivars->fd = 0;
- ErrMsg_set_with_errno("Can't open '%o'", ivars->path);
+ if (fd == -1) {
+ ErrMsg_set_with_errno("Attempt to open '%o' failed", path);
return false;
}
- // Derive len.
- ivars->len = chy_lseek64(ivars->fd, INT64_C(0), SEEK_END);
- if (ivars->len == -1) {
- ErrMsg_set_with_errno("lseek64 on %o failed", ivars->path);
- return false;
+ if (flags & FH_EXCLUSIVE) {
+ ivars->len = 0;
}
else {
- int64_t check_val = chy_lseek64(ivars->fd, INT64_C(0), SEEK_SET);
+ // Derive length.
+ ivars->len = chy_lseek64(fd, INT64_C(0), SEEK_END);
+ if (ivars->len == -1) {
+ ErrMsg_set_with_errno("lseek64 on %o failed", path);
+ return false;
+ }
+ int64_t check_val = chy_lseek64(fd, INT64_C(0), SEEK_SET);
if (check_val == -1) {
- ErrMsg_set_with_errno("lseek64 on %o failed", ivars->path);
+ ErrMsg_set_with_errno("lseek64 on %o failed", path);
return false;
}
}
- // Get system page size.
+ if (flags & FH_READ_ONLY) {
+ // Get system page size.
#if defined(_SC_PAGESIZE)
- ivars->page_size = sysconf(_SC_PAGESIZE);
+ ivars->page_size = sysconf(_SC_PAGESIZE);
#elif defined(_SC_PAGE_SIZE)
- ivars->page_size = sysconf(_SC_PAGE_SIZE);
+ ivars->page_size = sysconf(_SC_PAGE_SIZE);
#else
- #error "Can't determine system memory page size"
+ #error "Can't determine system memory page size"
#endif
+ }
+
+ ivars->fd = fd;
+
+ return true;
+}
+
+static bool
+S_close(FSFileHandleIVARS *ivars) {
+ // Close system-specific handles.
+ if (ivars->fd) {
+ if (close(ivars->fd)) {
+ ErrMsg_set_with_errno("Failed to close file '%o'", ivars->path);
+ return false;
+ }
+ ivars->fd = 0;
+ }
+ return true;
+}
+
+bool
+FSFH_Write_IMP(FSFileHandle *self, const void *data, size_t len) {
+ FSFileHandleIVARS *const ivars = FSFH_IVARS(self);
+
+ if (len) {
+ // Write data, track file length, check for errors.
+ int64_t check_val = write(ivars->fd, data, len);
+ if (check_val == -1) {
+ ErrMsg_set_with_errno("Error when writing %u64 bytes",
+ (uint64_t)len);
+ return false;
+ }
+
+ ivars->len += check_val;
+ if ((size_t)check_val != len) {
+ ErrMsg_set("Attempted to write %u64 bytes, but wrote %i64",
+ (uint64_t)len, check_val);
+ return false;
+ }
+ }
return true;
}
@@ -441,51 +381,112 @@ FSFH_Read_IMP(FSFileHandle *self, char *dest, int64_t offset, size_t len) {
#elif defined(CHY_HAS_WINDOWS_H)
-static CFISH_INLINE bool
-SI_init_read_only(FSFileHandle *self, FSFileHandleIVARS *ivars) {
- UNUSED_VAR(self);
- char *filepath = Str_To_Utf8(ivars->path);
- SYSTEM_INFO sys_info;
-
- // Get system page size.
- GetSystemInfo(&sys_info);
- ivars->page_size = sys_info.dwAllocationGranularity;
-
- // Open.
- ivars->win_fhandle = CreateFile(
- filepath,
- GENERIC_READ,
- FILE_SHARE_READ,
- NULL,
- OPEN_EXISTING,
- FILE_ATTRIBUTE_READONLY | FILE_FLAG_OVERLAPPED,
- NULL
- );
- FREEMEM(filepath);
- if (ivars->win_fhandle == INVALID_HANDLE_VALUE) {
- ErrMsg_set_with_win_error("CreateFile for %o failed", ivars->path);
+#include <windows.h>
+
+static bool
+S_init(FSFileHandleIVARS *ivars, String *path, uint32_t flags) {
+ DWORD desired_access = flags & FH_READ_ONLY
+ ? GENERIC_READ
+ : GENERIC_WRITE;
+ DWORD creation_disposition = flags & FH_CREATE
+ ? flags & FH_EXCLUSIVE
+ ? CREATE_NEW
+ : OPEN_ALWAYS
+ : OPEN_EXISTING;
+
+ char *path_ptr = Str_To_Utf8(path);
+ HANDLE handle
+ = CreateFileA(path_ptr, desired_access, FILE_SHARE_READ, NULL,
+ creation_disposition, FILE_ATTRIBUTE_NORMAL, NULL);
+ FREEMEM(path_ptr);
+
+ if (handle == INVALID_HANDLE_VALUE) {
+ ErrMsg_set_with_win_error("CreateFile for '%o' failed", path);
return false;
}
- // Derive len.
- DWORD file_size_hi;
- DWORD file_size_lo = GetFileSize(ivars->win_fhandle, &file_size_hi);
- if (file_size_lo == INVALID_FILE_SIZE && GetLastError() != NO_ERROR) {
- ErrMsg_set_with_win_error("GetFileSize for %o failed", ivars->path);
- return false;
+ if (flags & FH_EXCLUSIVE) {
+ ivars->len = 0;
+ }
+ else {
+ // Derive len.
+ DWORD file_size_hi;
+ DWORD file_size_lo = GetFileSize(handle, &file_size_hi);
+ if (file_size_lo == INVALID_FILE_SIZE && GetLastError() != NO_ERROR) {
+ ErrMsg_set_with_win_error("GetFileSize for '%o' failed", path);
+ return false;
+ }
+ ivars->len = ((uint64_t)file_size_hi << 32) | file_size_lo;
+ }
+
+ if (flags & FH_READ_ONLY) {
+ // Get system page size.
+ SYSTEM_INFO sys_info;
+ GetSystemInfo(&sys_info);
+ ivars->page_size = sys_info.dwAllocationGranularity;
+
+ // Init mapping handle.
+ if (ivars->len) {
+ ivars->win_maphandle
+ = CreateFileMapping(handle, NULL, PAGE_READONLY, 0, 0, NULL);
+ if (ivars->win_maphandle == NULL) {
+ ErrMsg_set_with_win_error("CreateFileMapping for '%o' failed",
+ path);
+ return false;
+ }
+ }
+ }
+
+ ivars->win_fhandle = handle;
+ return true;
+}
+
+static bool
+S_close(FSFileHandleIVARS *ivars) {
+ if (ivars->win_fhandle) {
+ // Close both standard handle and mapping handle.
+ if (ivars->win_maphandle) {
+ if (CloseHandle(ivars->win_maphandle) == 0) {
+ ErrMsg_set_with_win_error("CloseHandle for '%o' mapping"
+ " failed", ivars->path);
+ return false;
+ }
+ ivars->win_maphandle = NULL;
+ }
+
+ if (CloseHandle(ivars->win_fhandle) == 0) {
+ ErrMsg_set_with_win_error("CloseHandle for '%o' failed",
+ ivars->path);
+ return false;
+ }
+ ivars->win_fhandle = NULL;
}
- ivars->len = ((uint64_t)file_size_hi << 32) | file_size_lo;
- // Init mapping handle.
- ivars->buf = NULL;
- if (ivars->len) {
- ivars->win_maphandle = CreateFileMapping(ivars->win_fhandle, NULL,
- PAGE_READONLY, 0, 0, NULL);
- if (ivars->win_maphandle == NULL) {
- ErrMsg_set_with_win_error("CreateFileMapping for %o failed",
+ return true;
+}
+
+bool
+FSFH_Write_IMP(FSFileHandle *self, const void *vdata, size_t len) {
+ FSFileHandleIVARS *const ivars = FSFH_IVARS(self);
+ const char *data = (const char*)vdata;
+
+ while (len) {
+ // Write in 2GB chunks.
+ DWORD to_write = len > 0x80000000 ? 0x80000000 : len;
+ DWORD written = 0;
+
+ // Write data, track file length, check for errors.
+ BOOL result
+ = WriteFile(ivars->win_fhandle, data, to_write, &written, NULL);
+ ivars->len += written;
+ if (result == 0) {
+ ErrMsg_set_with_win_error("WriteFile for '%o' failed",
ivars->path);
return false;
}
+
+ data += to_write;
+ len -= to_write;
}
return true;
@@ -517,6 +518,7 @@ SI_map(FSFileHandle *self, FSFileHandleIVARS *ivars, int64_t offset,
static CFISH_INLINE bool
SI_unmap(FSFileHandle *self, char *buf, int64_t len) {
UNUSED_VAR(self);
+ UNUSED_VAR(len);
if (buf != NULL) {
if (!UnmapViewOfFile(buf)) {
ErrMsg_set_with_win_error("Failed to unmap '%o'",
@@ -527,40 +529,10 @@ SI_unmap(FSFileHandle *self, char *buf, int64_t len) {
return true;
}
-static CFISH_INLINE bool
-SI_close_win_handles(FSFileHandle *self) {
- FSFileHandleIVARS *ivars = FSFH_IVARS(self);
- // Close both standard handle and mapping handle.
- if (ivars->win_maphandle) {
- if (!CloseHandle(ivars->win_maphandle)) {
- ErrMsg_set_with_win_error("Failed to close file mapping handle");
- return false;
- }
- ivars->win_maphandle = NULL;
- }
- if (ivars->win_fhandle) {
- if (!CloseHandle(ivars->win_fhandle)) {
- ErrMsg_set_with_win_error("Failed to close file handle");
- return false;
- }
- ivars->win_fhandle = NULL;
- }
-
- return true;
-}
-
#if !IS_64_BIT
bool
FSFH_Read_IMP(FSFileHandle *self, char *dest, int64_t offset, size_t len) {
FSFileHandleIVARS *const ivars = FSFH_IVARS(self);
- BOOL check_val;
- DWORD got;
- OVERLAPPED read_op_state;
- uint64_t offs = (uint64_t)offset;
-
- read_op_state.hEvent = NULL;
- read_op_state.OffsetHigh = offs >> 32;
- read_op_state.Offset = offs & 0xFFFFFFFF;
// Sanity check.
if (offset < 0) {
@@ -568,33 +540,29 @@ FSFH_Read_IMP(FSFileHandle *self, char *dest, int64_t offset, size_t len) {
return false;
}
- // ReadFile() takes a DWORD (unsigned 32-bit integer) as a length
- // argument, so throw a sensible error rather than wrap around.
- if (len > UINT32_MAX) {
- ErrMsg_set("Can't read more than 4 GB (%u64)", (uint64_t)len);
- return false;
- }
-
- // Read.
- check_val = ReadFile(ivars->win_fhandle, dest, len, &got, &read_op_state);
- if (!check_val && GetLastError() == ERROR_IO_PENDING) {
- // Read has been queued by the OS and will soon complete. Wait for
- // it, since this is a blocking IO call from the point of the rest of
- // the library.
- check_val = GetOverlappedResult(ivars->win_fhandle, &read_op_state,
- &got, TRUE);
- }
+ DWORD got;
+ OVERLAPPED overlapped;
+ overlapped.hEvent = NULL;
+ overlapped.OffsetHigh = offset >> 32;
+ overlapped.Offset = offset & 0xFFFFFFFF;
- // Verify that the read has succeeded by now.
- if (!check_val) {
+ if (ReadFile(ivars->win_fhandle, dest, len, &got, &overlapped) == 0) {
ErrMsg_set_with_win_error("Failed to read %u64 bytes", (uint64_t)len);
return false;
}
+ if (got < len) {
+ ErrMsg_set("Tried to read %u64 bytes, got %u32", (uint64_t)len, got);
+ return false;
+ }
return true;
}
#endif // !IS_64_BIT
+#else // CHY_HAS_SYS_MMAN_H vs. CHY_HAS_WINDOWS_H
+
+#error "No support for memory mapped files"
+
#endif // CHY_HAS_SYS_MMAN_H vs. CHY_HAS_WINDOWS_H
http://git-wip-us.apache.org/repos/asf/lucy/blob/5f15a92b/core/Lucy/Store/FileHandle.c
----------------------------------------------------------------------
diff --git a/core/Lucy/Store/FileHandle.c b/core/Lucy/Store/FileHandle.c
index 276987e..017989e 100644
--- a/core/Lucy/Store/FileHandle.c
+++ b/core/Lucy/Store/FileHandle.c
@@ -18,18 +18,40 @@
#include "Lucy/Util/ToolSet.h"
#include "Lucy/Store/FileHandle.h"
+#include "Lucy/Store/ErrorMessage.h"
int32_t FH_object_count = 0;
FileHandle*
FH_do_open(FileHandle *self, String *path, uint32_t flags) {
+ // Track number of live FileHandles released into the wild.
+ FH_object_count++;
+
+ // Check flags.
+ uint32_t rw_flags = flags & (FH_READ_ONLY | FH_WRITE_ONLY);
+ if (rw_flags == 0) {
+ ErrMsg_set("Must specify FH_READ_ONLY or FH_WRITE_ONLY to open '%o'",
+ path);
+ DECREF(self);
+ return NULL;
+ }
+ else if (rw_flags == (FH_READ_ONLY | FH_WRITE_ONLY)) {
+ ErrMsg_set("Can't specify both FH_READ_ONLY and FH_WRITE_ONLY to"
+ " open '%o'", path);
+ DECREF(self);
+ return NULL;
+ }
+ if ((flags & (FH_CREATE | FH_EXCLUSIVE)) == FH_EXCLUSIVE) {
+ ErrMsg_set("Can't specify FH_EXCLUSIVE without FH_CREATE to"
+ " open '%o' ", path);
+ DECREF(self);
+ return NULL;
+ }
+
FileHandleIVARS *const ivars = FH_IVARS(self);
ivars->path = path ? Str_Clone(path) : Str_new_from_trusted_utf8("", 0);
ivars->flags = flags;
- // Track number of live FileHandles released into the wild.
- FH_object_count++;
-
ABSTRACT_CLASS_CHECK(self, FILEHANDLE);
return self;
}
http://git-wip-us.apache.org/repos/asf/lucy/blob/5f15a92b/core/Lucy/Store/RAMFileHandle.c
----------------------------------------------------------------------
diff --git a/core/Lucy/Store/RAMFileHandle.c b/core/Lucy/Store/RAMFileHandle.c
index c87c606..0768807 100644
--- a/core/Lucy/Store/RAMFileHandle.c
+++ b/core/Lucy/Store/RAMFileHandle.c
@@ -30,6 +30,9 @@ RAMFH_open(String *path, uint32_t flags, RAMFile *file) {
RAMFileHandle*
RAMFH_do_open(RAMFileHandle *self, String *path, uint32_t flags,
RAMFile *file) {
+ if (!FH_do_open((FileHandle*)self, path, flags)) { return NULL; }
+ RAMFileHandleIVARS *const ivars = RAMFH_IVARS(self);
+
bool must_create
= (flags & (FH_CREATE | FH_EXCLUSIVE)) == (FH_CREATE | FH_EXCLUSIVE)
? true : false;
@@ -37,9 +40,6 @@ RAMFH_do_open(RAMFileHandle *self, String *path, uint32_t flags,
= (flags & (FH_CREATE | FH_WRITE_ONLY)) == (FH_CREATE | FH_WRITE_ONLY)
? true : false;
- FH_do_open((FileHandle*)self, path, flags);
- RAMFileHandleIVARS *const ivars = RAMFH_IVARS(self);
-
// Obtain a RAMFile.
if (file) {
if (must_create) {
http://git-wip-us.apache.org/repos/asf/lucy/blob/5f15a92b/test/Lucy/Test/Store/MockFileHandle.c
----------------------------------------------------------------------
diff --git a/test/Lucy/Test/Store/MockFileHandle.c b/test/Lucy/Test/Store/MockFileHandle.c
index 029eb5f..2762afd 100644
--- a/test/Lucy/Test/Store/MockFileHandle.c
+++ b/test/Lucy/Test/Store/MockFileHandle.c
@@ -31,7 +31,7 @@ MockFileHandle_new(String *path, int64_t length) {
MockFileHandle*
MockFileHandle_init(MockFileHandle *self, String *path,
int64_t length) {
- FH_do_open((FileHandle*)self, path, 0);
+ if (!FH_do_open((FileHandle*)self, path, FH_READ_ONLY)) { return NULL; }
MockFileHandleIVARS *const ivars = MockFileHandle_IVARS(self);
ivars->len = length;
return self;
http://git-wip-us.apache.org/repos/asf/lucy/blob/5f15a92b/test/Lucy/Test/Store/TestFSFileHandle.c
----------------------------------------------------------------------
diff --git a/test/Lucy/Test/Store/TestFSFileHandle.c b/test/Lucy/Test/Store/TestFSFileHandle.c
index 518b71c..5e1c651 100644
--- a/test/Lucy/Test/Store/TestFSFileHandle.c
+++ b/test/Lucy/Test/Store/TestFSFileHandle.c
@@ -22,11 +22,7 @@
#define TESTLUCY_USE_SHORT_NAMES
#include "Lucy/Util/ToolSet.h"
-#ifdef CHY_HAS_UNISTD_H
- #include <unistd.h> // close
-#elif defined(CHY_HAS_IO_H)
- #include <io.h> // close
-#endif
+#include "charmony.h"
#include "Clownfish/TestHarness/TestBatchRunner.h"
#include "Lucy/Test.h"
@@ -159,6 +155,12 @@ test_Read_Write(TestBatchRunner *runner) {
"Read() past EOF sets error");
Err_set_error(NULL);
+ TEST_FALSE(runner, FSFH_Read(fh, buf, 4, 3),
+ "Read() across EOF returns false");
+ TEST_TRUE(runner, Err_get_error() != NULL,
+ "Read() across EOF sets error");
+
+ Err_set_error(NULL);
TEST_FALSE(runner, FSFH_Write(fh, foo, 3),
"Writing to a read-only handle returns false");
TEST_TRUE(runner, Err_get_error() != NULL,
@@ -184,8 +186,8 @@ test_Close(TestBatchRunner *runner) {
S_remove(test_filename);
fh = FSFH_open(test_filename,
FH_CREATE | FH_WRITE_ONLY | FH_EXCLUSIVE);
-#ifdef _MSC_VER
- SKIP(runner, 2, "LUCY-155");
+#if defined(CHY_HAS_WINDOWS_H) && !defined(CHY_HAS_SYS_MMAN_H)
+ SKIP(runner, 2, "Can't test invalid file handles on Windows");
#else
int saved_fd = FSFH_Set_FD(fh, -1);
Err_set_error(NULL);
@@ -194,7 +196,7 @@ test_Close(TestBatchRunner *runner) {
TEST_TRUE(runner, Err_get_error() != NULL,
"Failed Close() sets global error");
FSFH_Set_FD(fh, saved_fd);
-#endif /* _MSC_VER */
+#endif // Windows check.
DECREF(fh);
fh = FSFH_open(test_filename, FH_READ_ONLY);
@@ -260,7 +262,7 @@ test_Window(TestBatchRunner *runner) {
void
TestFSFH_Run_IMP(TestFSFileHandle *self, TestBatchRunner *runner) {
- TestBatchRunner_Plan(runner, (TestBatch*)self, 46);
+ TestBatchRunner_Plan(runner, (TestBatch*)self, 48);
test_open(runner);
test_Read_Write(runner);
test_Close(runner);
http://git-wip-us.apache.org/repos/asf/lucy/blob/5f15a92b/test/Lucy/Test/Store/TestFileHandle.c
----------------------------------------------------------------------
diff --git a/test/Lucy/Test/Store/TestFileHandle.c b/test/Lucy/Test/Store/TestFileHandle.c
index 2e9850f..739f856 100644
--- a/test/Lucy/Test/Store/TestFileHandle.c
+++ b/test/Lucy/Test/Store/TestFileHandle.c
@@ -46,7 +46,7 @@ S_new_filehandle() {
}
Class_Override(klass, S_no_op_method, LUCY_FH_Close_OFFSET);
fh = (FileHandle*)Class_Make_Obj(klass);
- return FH_do_open(fh, NULL, 0);
+ return FH_do_open(fh, NULL, FH_READ_ONLY);
}
void