susumu.yata
null+****@clear*****
Tue Jul 2 13:38:59 JST 2013
susumu.yata 2013-07-02 13:38:59 +0900 (Tue, 02 Jul 2013) New Revision: dcc49da8846ababad5c2ffc76c8d2d4573073875 https://github.com/groonga/grnxx/commit/dcc49da8846ababad5c2ffc76c8d2d4573073875 Message: Use grnxx::Exception. Modified files: lib/grnxx/errno.hpp lib/grnxx/storage/chunk-posix.cpp lib/grnxx/storage/file-posix.cpp lib/grnxx/storage/file-posix.hpp lib/grnxx/storage/file-windows.cpp lib/grnxx/storage/file-windows.hpp lib/grnxx/storage/file.hpp lib/grnxx/storage/storage_impl.cpp test/test_storage.cpp Modified: lib/grnxx/errno.hpp (+1 -0) =================================================================== --- lib/grnxx/errno.hpp 2013-07-01 18:59:48 +0900 (6ff15cf) +++ lib/grnxx/errno.hpp 2013-07-02 13:38:59 +0900 (74e8124) @@ -33,6 +33,7 @@ enum ErrnoType { class GRNXX_EXPORT Errno { public: + Errno() = default; // For errno. explicit Errno(int error_code) : type_(STANDARD_ERRNO), Modified: lib/grnxx/storage/chunk-posix.cpp (+2 -4) =================================================================== --- lib/grnxx/storage/chunk-posix.cpp 2013-07-01 18:59:48 +0900 (0047dcd) +++ lib/grnxx/storage/chunk-posix.cpp 2013-07-02 13:38:59 +0900 (492688a) @@ -112,10 +112,8 @@ uint64_t ChunkImpl::size() const { bool ChunkImpl::create_file_backed_chunk(File *file, uint64_t offset, uint64_t size, ChunkFlags flags) { - uint64_t file_size; - if (!file->get_size(&file_size)) { - return false; - } + // TODO: This may throw. + const uint64_t file_size = file->get_size(); if ((offset >= file_size) || (size > file_size) || (size > (file_size - offset))) { GRNXX_ERROR() << "invalid argument: offset = " << offset Modified: lib/grnxx/storage/file-posix.cpp (+70 -77) =================================================================== --- lib/grnxx/storage/file-posix.cpp 2013-07-01 18:59:48 +0900 (f7207f8) +++ lib/grnxx/storage/file-posix.cpp 2013-07-02 13:38:59 +0900 (e7c257d) @@ -30,6 +30,7 @@ #include <new> #include "grnxx/errno.hpp" +#include "grnxx/exception.hpp" #include "grnxx/logger.hpp" #include "grnxx/storage/path.hpp" @@ -54,8 +55,9 @@ FileImpl::~FileImpl() { unlock(); } if (::close(fd_) != 0) { - GRNXX_ERROR() << "failed to close file: path = " << path_.get() - << ": '::close' " << Errno(errno); + Errno errno_copy(errno); + GRNXX_WARNING() << "failed to close file: path = " << path_.get() + << ", call = ::close, errno = " << errno_copy; } } } @@ -64,16 +66,12 @@ FileImpl *FileImpl::create(const char *path, FileFlags flags) { std::unique_ptr<FileImpl> file(new (std::nothrow) FileImpl); if (!file) { GRNXX_ERROR() << "new grnxx::storage::FileImpl failed"; - return nullptr; + throw MemoryError(); } if (path && (~flags & FILE_TEMPORARY)) { - if (!file->create_persistent_file(path, flags)) { - return nullptr; - } + file->create_persistent_file(path, flags); } else { - if (!file->create_temporary_file(path, flags)) { - return nullptr; - } + file->create_temporary_file(path, flags); } return file.release(); } @@ -81,42 +79,43 @@ FileImpl *FileImpl::create(const char *path, FileFlags flags) { FileImpl *FileImpl::open(const char *path, FileFlags flags) { if (!path) { GRNXX_ERROR() << "invalid argument: path = nullptr"; - return nullptr; + throw LogicError(); } std::unique_ptr<FileImpl> file(new (std::nothrow) FileImpl); if (!file) { GRNXX_ERROR() << "new grnxx::storage::FileImpl failed"; - return nullptr; - } - if (!file->open_file(path, flags)) { - return nullptr; + throw MemoryError(); } + file->open_file(path, flags); return file.release(); } FileImpl *FileImpl::open_or_create(const char *path, FileFlags flags) { if (!path) { GRNXX_ERROR() << "invalid argument: path = nullptr"; - return nullptr; + throw LogicError(); } std::unique_ptr<FileImpl> file(new (std::nothrow) FileImpl); if (!file) { GRNXX_ERROR() << "new grnxx::storage::FileImpl failed"; - return nullptr; - } - if (!file->open_or_create_file(path, flags)) { - return nullptr; + throw MemoryError(); } + file->open_or_create_file(path, flags); return file.release(); } bool FileImpl::exists(const char *path) { if (!path) { GRNXX_ERROR() << "invalid argument: path = nullptr"; - return false; + throw LogicError(); } struct stat stat; if (::stat(path, &stat) != 0) { + if (errno != ENOENT) { + Errno errno_copy(errno); + GRNXX_WARNING() << "failed to get file information: " + << "call = ::stat, errno = " << errno_copy; + } return false; } return S_ISREG(stat.st_mode); @@ -125,11 +124,15 @@ bool FileImpl::exists(const char *path) { bool FileImpl::unlink(const char *path) { if (!path) { GRNXX_ERROR() << "invalid argument: path = nullptr"; + throw LogicError(); + } + if (!exists(path)) { return false; } if (::unlink(path) != 0) { + Errno errno_copy(errno); GRNXX_WARNING() << "failed to unlink file: path = " << path - << ": '::unlink' " << Errno(errno); + << ", call = ::unlink, errno = " << errno_copy; return false; } return true; @@ -138,13 +141,13 @@ bool FileImpl::unlink(const char *path) { bool FileImpl::lock(FileLockFlags lock_flags) { if (locked_) { GRNXX_ERROR() << "already locked: path = " << path_.get(); - return false; + throw LogicError(); } FileLockFlags lock_type = lock_flags & (FILE_LOCK_SHARED | FILE_LOCK_EXCLUSIVE); if (!lock_type || (lock_type == (FILE_LOCK_SHARED | FILE_LOCK_EXCLUSIVE))) { GRNXX_ERROR() << "invalid argument: lock_flags == " << lock_flags; - return false; + throw LogicError(); } int operation = 0; if (lock_flags & FILE_LOCK_SHARED) { @@ -158,70 +161,68 @@ bool FileImpl::lock(FileLockFlags lock_flags) { } if (::flock(fd_, operation) != 0) { if (errno == EWOULDBLOCK) { - // The file is locked by others. return false; } + Errno errno_copy(errno); GRNXX_ERROR() << "failed to lock file: path = " << path_.get() << ", lock_flags = " << lock_flags - << ": '::flock' " << Errno(errno); - return false; + << ", call = ::flock, errno = " << Errno(errno_copy); + throw SystemError(errno_copy); } locked_ = true; return true; } -bool FileImpl::unlock() { +void FileImpl::unlock() { if (!locked_) { GRNXX_WARNING() << "unlocked: path = " << path_.get(); - return false; + throw LogicError(); } if (::flock(fd_, LOCK_UN) != 0) { + Errno errno_copy(errno); GRNXX_ERROR() << "failed to unlock file: path = " << path_.get() - << ": '::flock' " << Errno(errno); - return false; + << ", call = ::flock, errno = " << errno_copy; + throw SystemError(errno_copy); } locked_ = false; - return true; } -bool FileImpl::sync() { +void FileImpl::sync() { if (::fsync(fd_) != 0) { + Errno errno_copy(errno); GRNXX_ERROR() << "failed to sync file: path = " << path_.get() - << ": '::fsync' " << Errno(errno); - return false; + << ", call = ::fsync, errno = " << errno_copy; + throw SystemError(errno_copy); } - return true; } -bool FileImpl::resize(uint64_t size) { +void FileImpl::resize(uint64_t size) { if (flags_ & FILE_READ_ONLY) { - GRNXX_ERROR() << "read-only"; - return false; + GRNXX_ERROR() << "invalid operation: flags = " << flags_; + throw LogicError(); } if (size > static_cast<uint64_t>(std::numeric_limits<off_t>::max())) { GRNXX_ERROR() << "invalid argument: size = " << size; - return false; + throw LogicError(); } if (::ftruncate(fd_, size) != 0) { + Errno errno_copy(errno); GRNXX_ERROR() << "failed to resize file: path = " << path_.get() << ", size = " << size - << ": '::ftruncate' " << Errno(errno); - return false; + << ", call = ::ftruncate, errno = " << errno_copy; + throw SystemError(errno_copy); } - return true; } -bool FileImpl::get_size(uint64_t *size) { +uint64_t FileImpl::get_size() { struct stat stat; if (::fstat(fd_, &stat) != 0) { + Errno errno_copy(errno); GRNXX_ERROR() << "failed to stat file: path = " << path_.get() - << ": '::fstat' " << Errno(errno); - return false; - } - if (size) { - *size = stat.st_size; + << ", call = ::fstat, errno = " << errno_copy; + throw SystemError(errno_copy); } - return true; + return stat.st_size; } const char *FileImpl::path() const { @@ -236,26 +237,23 @@ const void *FileImpl::handle() const { return &fd_; } -bool FileImpl::create_persistent_file(const char *path, FileFlags flags) { +void FileImpl::create_persistent_file(const char *path, FileFlags flags) { if (!path) { GRNXX_ERROR() << "invalid argument: path = nullptr"; - return false; + throw LogicError(); } path_.reset(Path::clone_path(path)); - if (!path_) { - return false; - } fd_ = ::open(path, O_RDWR | O_CREAT | O_EXCL, 0644); if (fd_ == -1) { + Errno errno_copy(errno); GRNXX_ERROR() << "failed to create file: path = " << path << ", flags = " << flags - << ": '::open' " << Errno(errno); - return false; + << ", call = ::open, errno = " << errno_copy; + throw SystemError(errno_copy); } - return true; } -bool FileImpl::create_temporary_file(const char *path, FileFlags flags) { +void FileImpl::create_temporary_file(const char *path, FileFlags flags) { flags_ = FILE_TEMPORARY; int posix_flags = O_RDWR | O_CREAT | O_EXCL | O_NOCTTY; #ifdef O_NOATIME @@ -264,27 +262,25 @@ bool FileImpl::create_temporary_file(const char *path, FileFlags flags) { #ifdef O_NOFOLLOW posix_flags |= O_NOFOLLOW; #endif // O_NOFOLLOW + Errno errno_copy; for (int i = 0; i < UNIQUE_PATH_GENERATION_TRIAL_COUNT; ++i) { path_.reset(Path::unique_path(path)); fd_ = ::open(path_.get(), posix_flags, 0600); if (fd_ != -1) { unlink(path_.get()); - return true; + return; } + errno_copy = Errno(errno); GRNXX_WARNING() << "failed to create file: path = " << path_.get() - << ": '::open' " << Errno(errno); + << ", call = ::open, errno = " << errno_copy; } GRNXX_ERROR() << "failed to create temporary file: " - << "path = " << path - << ", flags = " << flags; - return false; + << "path = " << path << ", flags = " << flags; + throw SystemError(errno_copy); } -bool FileImpl::open_file(const char *path, FileFlags flags) { +void FileImpl::open_file(const char *path, FileFlags flags) { path_.reset(Path::clone_path(path)); - if (!path_) { - return false; - } int posix_flags = O_RDWR; if (flags & FILE_READ_ONLY) { posix_flags = O_RDONLY; @@ -292,27 +288,24 @@ bool FileImpl::open_file(const char *path, FileFlags flags) { } fd_ = ::open(path, posix_flags); if (fd_ == -1) { + Errno errno_copy(errno); GRNXX_ERROR() << "failed to open file: path = " << path << ", flags = " << flags - << ": '::open' " << Errno(errno); - return false; + << ", call = ::open, errno = " << errno_copy; + throw SystemError(errno_copy); } - return true; } -bool FileImpl::open_or_create_file(const char *path, FileFlags flags) { +void FileImpl::open_or_create_file(const char *path, FileFlags flags) { path_.reset(Path::clone_path(path)); - if (!path_) { - return false; - } fd_ = ::open(path, O_RDWR | O_CREAT, 0644); if (fd_ == -1) { + Errno errno_copy(errno); GRNXX_ERROR() << "failed to open file: path = " << path << ", flags = " << flags - << ": '::open' " << Errno(errno); - return false; + << ", call = ::open, errno = " << errno_copy; + throw SystemError(errno_copy); } - return true; } } // namespace storage Modified: lib/grnxx/storage/file-posix.hpp (+8 -8) =================================================================== --- lib/grnxx/storage/file-posix.hpp 2013-07-01 18:59:48 +0900 (323da5d) +++ lib/grnxx/storage/file-posix.hpp 2013-07-02 13:38:59 +0900 (3cfc728) @@ -42,12 +42,12 @@ class FileImpl : public File { static bool unlink(const char *path); bool lock(FileLockFlags lock_flags); - bool unlock(); + void unlock(); - bool sync(); + void sync(); - bool resize(uint64_t size); - bool get_size(uint64_t *size); + void resize(uint64_t size); + uint64_t get_size(); const char *path() const; FileFlags flags() const; @@ -59,10 +59,10 @@ class FileImpl : public File { int fd_; bool locked_; - bool create_persistent_file(const char *path, FileFlags flags); - bool create_temporary_file(const char *path_prefix, FileFlags flags); - bool open_file(const char *path, FileFlags flags); - bool open_or_create_file(const char *path, FileFlags flags); + void create_persistent_file(const char *path, FileFlags flags); + void create_temporary_file(const char *path_prefix, FileFlags flags); + void open_file(const char *path, FileFlags flags); + void open_or_create_file(const char *path, FileFlags flags); }; } // namespace storage Modified: lib/grnxx/storage/file-windows.cpp (+74 -78) =================================================================== --- lib/grnxx/storage/file-windows.cpp 2013-07-01 18:59:48 +0900 (15f01d0) +++ lib/grnxx/storage/file-windows.cpp 2013-07-02 13:38:59 +0900 (41317cc) @@ -23,10 +23,12 @@ #include <sys/stat.h> #include <io.h> +#include <cerrno> #include <limits> #include <new> #include "grnxx/errno.hpp" +#include "grnxx/exception.hpp" #include "grnxx/logger.hpp" #include "grnxx/storage/path.hpp" @@ -51,8 +53,9 @@ FileImpl::~FileImpl() { unlock(); } if (!::CloseHandle(handle_)) { - GRNXX_ERROR() << "failed to close file: file = " << *this - << ": '::CloseHandle' " << Errno(::GetLastError()); + Errno errno_copy(::GetLastError()); + GRNXX_WARNING() << "failed to close file: file = " << *this + << ", call = ::CloseHandle, errno = " << errno_copy; } } } @@ -62,16 +65,12 @@ FileImpl *FileImpl::create(const char *path, FileFlags flags) { std::unique_ptr<FileImpl> file(new (std::nothrow) FileImpl); if (!file) { GRNXX_ERROR() << "new grnxx::storage::FileImpl failed"; - return nullptr; + throw MemoryError(); } if (path && (~flags & FILE_TEMPORARY)) { - if (!file->create_persistent_file(path, flags)) { - return nullptr; - } + file->create_persistent_file(path, flags); } else { - if (!file->create_temporary_file(path, flags)) { - return nullptr; - } + file->create_temporary_file(path, flags); } return file.release(); } @@ -79,42 +78,43 @@ FileImpl *FileImpl::create(const char *path, FileFlags flags) { FileImpl *FileImpl::open(const char *path, FileFlags flags) { if (!path) { GRNXX_ERROR() << "invalid argument: path = nullptr"; - return nullptr; + throw LogicError(); } std::unique_ptr<FileImpl> file(new (std::nothrow) FileImpl); if (!file) { GRNXX_ERROR() << "new grnxx::storage::FileImpl failed"; - return nullptr; - } - if (!file->open_file(path, flags)) { - return nullptr; + throw MemoryError(); } + file->open_file(path, flags); return file.release(); } FileImpl *FileImpl::open_or_create(const char *path, FileFlags flags) { if (!path) { GRNXX_ERROR() << "invalid argument: path = nullptr"; - return nullptr; + throw LogicError(); } std::unique_ptr<FileImpl> file(new (std::nothrow) FileImpl); if (!file) { GRNXX_ERROR() << "new grnxx::storage::FileImpl failed"; - return nullptr; - } - if (!file->open_or_create_file(path, flags)) { - return nullptr; + throw MemoryError(); } + file->open_or_create_file(path, flags); return file.release(); } bool FileImpl::exists(const char *path) { if (!path) { GRNXX_ERROR() << "invalid argument: path = nullptr"; - return false; + throw LogicError(); } struct _stat stat; if (::_stat(path, &stat) != 0) { + if (errno != ENOENT) { + Errno errno_copy(errno); + GRNXX_WARNING() << "failed to get file information: " + << "call = ::_stat, errno = " << errno_copy; + } return false; } return stat.st_mode & _S_IFREG; @@ -123,26 +123,30 @@ bool FileImpl::exists(const char *path) { bool FileImpl::unlink(const char *path) { if (!path) { GRNXX_ERROR() << "invalid argument: path = nullptr"; + throw LogicError(); + } + if (!exists(path)) { return false; } if (!::DeleteFile(path)) { + Errno errno_copy(::GetLastError()); GRNXX_WARNING() << "failed to unlink file: path = " << path - << ": '::DeleteFile' " << Errno(::GetLastError()); + << ", call = ::DeleteFile, errno = " << errno_copy; return false; } return true; } -bool FileImpl::try_lock(FileLockMode mode) { +bool FileImpl::lock(FileLockMode mode) { if (locked_) { GRNXX_ERROR() << "already locked: path = " << path_.get(); - return false; + throw LogicError(); } FileLockFlags lock_type = lock_flags & (FILE_LOCK_SHARED | FILE_LOCK_EXCLUSIVE); if (!lock_type || (lock_type == (FILE_LOCK_SHARED | FILE_LOCK_EXCLUSIVE))) { GRNXX_ERROR() << "invalid argument: lock_flags == " << lock_flags; - return false; + throw LogicError(); } DWORD flags = LOCKFILE_FAIL_IMMEDIATELY; if (lock_flags & FILE_LOCK_SHARED) { @@ -164,79 +168,78 @@ bool FileImpl::try_lock(FileLockMode mode) { // The file is locked by others. return false; } + Errno errno_copy(last_error); GRNXX_ERROR() << "failed to lock file: path = " << path_.get() << ", mode = " << mode - << ": '::LockFileEx' " << Errno(last_error); - return false; + << ", call = ::LockFileEx, errno = " << errno_copy; + throw SystemError(errno_copy); } locked_ = true; return true; } -bool FileImpl::unlock() { +void FileImpl::unlock() { if (!locked_) { GRNXX_WARNING() << "unlocked: path = " << path_.get(); - return false; + throw LogicError(); } - OVERLAPPED overlapped; overlapped.Offset = 0; overlapped.OffsetHigh = 0x80000000U; if (!::UnlockFileEx(handle_, 0, 0, 0x80000000U, &overlapped)) { + Errno errno_copy(::GetLastError()); GRNXX_ERROR() << "failed to unlock file: path = " << path_.get() - << ": '::UnlockFileEx' " << Errno(::GetLastError()); - return false; + << ", call = ::UnlockFileEx, errno = " << errno_copy; + throw SystemError(errno_copy); } locked_ = false; - return true; } void FileImpl::sync() { if (!::FlushFileBuffers(handle_)) { + Errno errno_copy(::GetLastError()); GRNXX_ERROR() << "failed to sync file: path = " << path_.get() - << ": '::FlushFileBuffers' " << Errno(::GetLastError()); - return false; + << ", call = ::FlushFileBuffers, errno = " << errno_copy; + throw SystemError(errno_copy); } - return true; } -bool FileImpl::resize(uint64_t size) { +void FileImpl::resize(uint64_t size) { if (flags_ & FILE_READ_ONLY) { - GRNXX_ERROR() << "read-only"; - return false; + GRNXX_ERROR() << "invalid operation: flags = " << flags_; + throw LogicError(); } if (size > static_cast<uint64_t>(std::numeric_limits<LONGLONG>::max())) { GRNXX_ERROR() << "invalid argument: size = " << size; - return false; + throw LogicError(); } LARGE_INTEGER request; request.QuadPart = size; if (!::SetFilePointerEx(handle_, request, nullptr, FILE_BEGIN)) { + Errno errno_copy(::GetLastError()); GRNXX_ERROR() << "failed to seek file: path = " << path_.get() << ", offset = " << offset << ", whence = " << whence - << ": '::SetFilePointerEx' " << Errno(::GetLastError()); - return false; + << ", call = ::SetFilePointerEx, errno = " << errno_copy; + throw SystemError(errno_copy); } if (!::SetEndOfFile(handle_)) { + Errno errno_copy(::GetLastError()); GRNXX_ERROR() << "failed to resize file: path = " << path_.get() << ", size = " << size - << ": '::SetEndOfFile' " << Errno(::GetLastError()); - return false; + << ", call = ::SetEndOfFile, errno = " << errno_copy; + throw SystemError(errno_copy); } - return true; } -bool FileImpl::get_size(uint64_t *size) { +uint64_t FileImpl::get_size() { LARGE_INTEGER file_size; if (!::GetFileSizeEx(handle_, &file_size)) { + Errno errno_copy(::GetLastError()); GRNXX_ERROR() << "failed to get file size: path = " << path_.get() << ": '::GetFileSizeEx' " << Errno(::GetLastError()); - return false; + throw SystemError(errno_copy); } - if (size) { - *size = file_size.QuadPart; - } - return true; + return file_size.QuadPart; } const char *FileImpl::path() const { @@ -251,15 +254,12 @@ const void *FileImpl::handle() const { return &handle_; } -bool FileImpl::create_persistent_file(const char *path, FileFlags flags) { +void FileImpl::create_persistent_file(const char *path, FileFlags flags) { if (!path) { GRNXX_ERROR() << "invalid argument: path = nullptr"; - return false; + throw LogicError(); } path_.reset(Path::clone_path(path)); - if (!path_) { - return false; - } const DWORD desired_access = GENERIC_READ | GENERIC_WRITE; const DWORD share_mode = FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE; @@ -268,42 +268,41 @@ bool FileImpl::create_persistent_file(const char *path, FileFlags flags) { handle_ = ::CreateFileA(path, desired_access, share_mode, nullptr, creation_disposition, flags_and_attributes, nullptr); if (handle_ == INVALID_HANDLE_VALUE) { + Errno errno_copy(::GetLastError()); GRNXX_ERROR() << "failed to open file: path = " << path << ", flags = " << flags - << ": '::CreateFileA' " << Errno(::GetLastError()); - return false; + << ", call = ::CreateFileA, errno = " << errno_copy; + throw SystemError(errno_copy); } - return true; } -bool FileImpl::create_temporary_file(const char *path, FileFlags flags) { +void FileImpl::create_temporary_file(const char *path, FileFlags flags) { flags_ = FILE_TEMPORARY; const DWORD desired_access = GENERIC_READ | GENERIC_WRITE; const DWORD share_mode = FILE_SHARE_DELETE; const DWORD creation_disposition = CREATE_NEW; const DWORD flags_and_attributes = FILE_ATTRIBUTE_TEMPORARY | FILE_FLAG_DELETE_ON_CLOSE; + Errno errno_copy; for (int i = 0; i < UNIQUE_PATH_GENERATION_TRIAL_COUNT; ++i) { path_.reset(Path::unique_path(path)); handle_ = ::CreateFileA(path_.get(), desired_access, share_mode, nullptr, creation_disposition, flags_and_attributes, nullptr); if (handle_ != INVALID_HANDLE_VALUE) { - return true; + return; } + errno_copy = Errno(::GetLastError()); GRNXX_WARNING() << "failed to create file: path = " << path_.get() - << ": '::CreateFileA' " << Errno(::GetLastError()); + << ", call = ::CreateFileA, errno = " << errno_copy; } - GRNXX_ERROR() << "failed to create temporary file: path = " << path - << ", flags = " << flags; - return false; + GRNXX_ERROR() << "failed to create temporary file: " + << "path = " << path << ", flags = " << flags; + throw SystemError(errno_copy); } -bool FileImpl::open_file(const char *path, FileFlags flags) { +void FileImpl::open_file(const char *path, FileFlags flags) { path_.reset(Path::clone_path(path)); - if (!path_) { - return false; - } DWORD desired_access = GENERIC_READ | GENERIC_WRITE; if (flags_ & FILE_READ_ONLY) { flags_ |= FILE_READ_ONLY; @@ -316,19 +315,16 @@ bool FileImpl::open_file(const char *path, FileFlags flags) { handle_ = ::CreateFileA(path, desired_access, share_mode, nullptr, creation_disposition, flags_and_attributes, nullptr); if (handle_ == INVALID_HANDLE_VALUE) { + Errno errno_copy(::GetLastError()); GRNXX_ERROR() << "failed to open file: path = " << path << ", flags = " << flags - << ": '::CreateFileA' " << Errno(::GetLastError()); - return false; + << ", call = ::CreateFileA, errno = " << errno_copy; + throw SystemError(errno_copy); } - return true; } -bool FileImpl::open_or_create_file(const char *path, FileFlags flags) { +void FileImpl::open_or_create_file(const char *path, FileFlags flags) { path_.reset(Path::clone_path(path)); - if (!path_) { - return false; - } const DWORD desired_access = GENERIC_READ | GENERIC_WRITE; const DWORD share_mode = FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE; @@ -337,12 +333,12 @@ bool FileImpl::open_or_create_file(const char *path, FileFlags flags) { handle_ = ::CreateFileA(path, desired_access, share_mode, nullptr, creation_disposition, flags_and_attributes, nullptr); if (handle_ == INVALID_HANDLE_VALUE) { + Errno errno_copy(::GetLastError()); GRNXX_ERROR() << "failed to open file: path = " << path << ", flags = " << flags - << ": '::CreateFileA' " << Errno(::GetLastError()); - return false; + << ", call = ::CreateFileA, errno = " << errno_copy; + throw SystemError(errno_copy); } - return true; } } // namespace storage Modified: lib/grnxx/storage/file-windows.hpp (+8 -8) =================================================================== --- lib/grnxx/storage/file-windows.hpp 2013-07-01 18:59:48 +0900 (89f5de4) +++ lib/grnxx/storage/file-windows.hpp 2013-07-02 13:38:59 +0900 (a5efd8e) @@ -49,12 +49,12 @@ class FileImpl : public File { static bool unlink(const char *path); bool lock(FileLockFlags lock_flags); - bool unlock(); + void unlock(); - bool sync(); + void sync(); - bool resize(uint64_t size); - bool get_size(uint64_t *size); + void resize(uint64_t size); + uint64_t get_size(); const char *path() const; FileFlags flags() const; @@ -67,10 +67,10 @@ class FileImpl : public File { int fd_; bool locked_; - bool create_persistent_file(const char *path, FileFlags flags); - bool create_temporary_file(const char *path_prefix, FileFlags flags); - bool open_file(const char *path, FileFlags flags); - bool open_or_create_file(const char *path, FileFlags flags); + void create_persistent_file(const char *path, FileFlags flags); + void create_temporary_file(const char *path_prefix, FileFlags flags); + void open_file(const char *path, FileFlags flags); + void open_or_create_file(const char *path, FileFlags flags); }; } // namespace storage Modified: lib/grnxx/storage/file.hpp (+5 -5) =================================================================== --- lib/grnxx/storage/file.hpp 2013-07-01 18:59:48 +0900 (d340f61) +++ lib/grnxx/storage/file.hpp 2013-07-02 13:38:59 +0900 (00b46d0) @@ -81,17 +81,17 @@ class File { // Try to lock a file and return true on success. // Note that the file is accessible even if it is locked (advisory). virtual bool lock(FileLockFlags lock_flags) = 0; - // Unlock a file and return true on success. - virtual bool unlock() = 0; + // Unlock a file. + virtual void unlock() = 0; // Flush modified pages and return true on success. - virtual bool sync() = 0; + virtual void sync() = 0; // Extend or truncate a file to "size" bytes. // Note that the contents of the extended part are undefined. - virtual bool resize(uint64_t size) = 0; + virtual void resize(uint64_t size) = 0; // Return the file size on success, or a negative value on failure. - virtual bool get_size(uint64_t *size) = 0; + virtual uint64_t get_size() = 0; // Return the file path. virtual const char *path() const = 0; Modified: lib/grnxx/storage/storage_impl.cpp (+10 -16) =================================================================== --- lib/grnxx/storage/storage_impl.cpp 2013-07-01 18:59:48 +0900 (b5749af) +++ lib/grnxx/storage/storage_impl.cpp 2013-07-02 13:38:59 +0900 (f1d6abe) @@ -425,9 +425,8 @@ bool StorageImpl::create_file_backed_storage(const char *path, if (!header_file) { return false; } - if (!header_file->resize(ROOT_CHUNK_SIZE)) { - return false; - } + // TODO: This may throw. + header_file->resize(ROOT_CHUNK_SIZE); std::unique_ptr<Chunk> root_chunk( create_chunk(header_file.get(), 0, ROOT_CHUNK_SIZE)); if (!root_chunk) { @@ -547,9 +546,8 @@ bool StorageImpl::open_or_create_storage(const char *path, StorageFlags flags, if (!header_file) { return false; } - if (!header_file->resize(ROOT_CHUNK_SIZE)) { - return false; - } + // TODO: This may throw. + header_file->resize(ROOT_CHUNK_SIZE); std::unique_ptr<Chunk> root_chunk( create_chunk(header_file.get(), 0, ROOT_CHUNK_SIZE)); if (!root_chunk) { @@ -1140,19 +1138,15 @@ File *StorageImpl::reserve_file(uint16_t file_id, uint64_t size) { } } // Expand the file if its size is not enough - uint64_t file_size; - if (!files_[file_id]->get_size(&file_size)) { - return nullptr; - } + // TODO: This may throw. + uint64_t file_size = files_[file_id]->get_size(); if (file_size < size) { Lock file_lock(&header_->file_mutex); - if (!files_[file_id]->get_size(&file_size)) { - return nullptr; - } + // TODO: This may throw. + file_size = files_[file_id]->get_size(); if (file_size < size) { - if (!files_[file_id]->resize(size)) { - return nullptr; - } + // TODO: This may throw. + files_[file_id]->resize(size); } } return files_[file_id].get(); Modified: test/test_storage.cpp (+18 -45) =================================================================== --- test/test_storage.cpp 2013-07-01 18:59:48 +0900 (e1516a5) +++ test/test_storage.cpp 2013-07-02 13:38:59 +0900 (f086d29) @@ -34,17 +34,14 @@ namespace { void test_full_path(const char *path, const char *answer) { std::unique_ptr<char[]> full_path(grnxx::storage::Path::full_path(path)); - assert(full_path); assert(std::strcmp(full_path.get(), answer) == 0); } void test_full_path() { std::unique_ptr<char[]> full_path(grnxx::storage::Path::full_path(nullptr)); - assert(full_path); GRNXX_NOTICE() << "full_path = " << full_path.get(); full_path.reset(grnxx::storage::Path::full_path("temp.grn")); - assert(full_path); GRNXX_NOTICE() << "full_path = " << full_path.get(); test_full_path("/", "/"); @@ -62,7 +59,6 @@ void test_full_path() { void test_unique_path() { std::unique_ptr<char[]> unique_path( grnxx::storage::Path::unique_path(nullptr)); - assert(unique_path); GRNXX_NOTICE() << "unique_path = " << unique_path.get(); unique_path.reset(grnxx::storage::Path::unique_path("temp.grn")); @@ -75,21 +71,14 @@ void test_file_create() { std::unique_ptr<grnxx::storage::File> file; file.reset(grnxx::storage::File::create(FILE_PATH)); - assert(file); - file.reset(grnxx::storage::File::create(FILE_PATH)); - assert(!file); file.reset(grnxx::storage::File::create(FILE_PATH, grnxx::storage::FILE_TEMPORARY)); - assert(file); file.reset(grnxx::storage::File::create(FILE_PATH, grnxx::storage::FILE_TEMPORARY)); - assert(file); file.reset(grnxx::storage::File::create(nullptr)); - assert(file); file.reset(grnxx::storage::File::create(nullptr)); - assert(file); grnxx::storage::File::unlink(FILE_PATH); } @@ -99,12 +88,8 @@ void test_file_open() { grnxx::storage::File::unlink(FILE_PATH); std::unique_ptr<grnxx::storage::File> file; - file.reset(grnxx::storage::File::open(FILE_PATH)); - assert(!file); - file.reset(grnxx::storage::File::create(FILE_PATH)); file.reset(grnxx::storage::File::open(FILE_PATH)); - assert(file); file.reset(); grnxx::storage::File::unlink(FILE_PATH); @@ -116,9 +101,7 @@ void test_file_open_or_create() { std::unique_ptr<grnxx::storage::File> file; file.reset(grnxx::storage::File::open_or_create(FILE_PATH)); - assert(file); file.reset(grnxx::storage::File::open_or_create(FILE_PATH)); - assert(file); file.reset(); grnxx::storage::File::unlink(FILE_PATH); @@ -131,25 +114,20 @@ void test_file_exists_and_unlink() { assert(grnxx::storage::File::exists(FILE_PATH)); assert(grnxx::storage::File::unlink(FILE_PATH)); - assert(!grnxx::storage::File::unlink(FILE_PATH)); assert(!grnxx::storage::File::exists(FILE_PATH)); + assert(!grnxx::storage::File::unlink(FILE_PATH)); } void test_file_lock_and_unlock() { const char FILE_PATH[] = "temp.grn"; std::unique_ptr<grnxx::storage::File> file_1; file_1.reset(grnxx::storage::File::open_or_create(FILE_PATH)); - assert(file_1); assert(file_1->lock(grnxx::storage::FILE_LOCK_SHARED)); - assert(!file_1->lock(grnxx::storage::FILE_LOCK_SHARED)); - assert(file_1->unlock()); - assert(!file_1->unlock()); + file_1->unlock(); assert(file_1->lock(grnxx::storage::FILE_LOCK_EXCLUSIVE)); - assert(!file_1->lock(grnxx::storage::FILE_LOCK_EXCLUSIVE)); - assert(file_1->unlock()); - assert(!file_1->unlock()); + file_1->unlock(); std::unique_ptr<grnxx::storage::File> file_2; file_2.reset(grnxx::storage::File::open(FILE_PATH)); @@ -158,17 +136,17 @@ void test_file_lock_and_unlock() { assert(file_1->lock(grnxx::storage::FILE_LOCK_SHARED)); assert(file_2->lock(grnxx::storage::FILE_LOCK_SHARED | grnxx::storage::FILE_LOCK_NONBLOCKING)); - assert(file_2->unlock()); + file_2->unlock(); assert(!file_2->lock(grnxx::storage::FILE_LOCK_EXCLUSIVE | grnxx::storage::FILE_LOCK_NONBLOCKING)); - assert(file_1->unlock()); + file_1->unlock(); assert(file_1->lock(grnxx::storage::FILE_LOCK_EXCLUSIVE)); assert(!file_2->lock(grnxx::storage::FILE_LOCK_SHARED | grnxx::storage::FILE_LOCK_NONBLOCKING)); assert(!file_2->lock(grnxx::storage::FILE_LOCK_EXCLUSIVE | grnxx::storage::FILE_LOCK_NONBLOCKING)); - assert(file_1->unlock()); + file_1->unlock(); file_1.reset(); file_2.reset(); @@ -180,24 +158,19 @@ void test_file_sync() { grnxx::storage::File::create(nullptr)); assert(file); - assert(file->sync()); + file->sync(); } void test_file_resize_and_size() { std::unique_ptr<grnxx::storage::File> file( grnxx::storage::File::create(nullptr)); - std::uint64_t file_size; assert(file); - assert(file->get_size(&file_size)); - assert(file_size == 0); - assert(file->resize(65536)); - assert(file->get_size(&file_size)); - assert(file_size == 65536); - assert(file->resize(1024)); - assert(file->get_size(&file_size)); - assert(file_size == 1024); - assert(!file->resize(-1)); + assert(file->get_size() == 0); + file->resize(65536); + assert(file->get_size() == 65536); + file->resize(1024); + assert(file->get_size() == 1024); } void test_file_path() { @@ -257,7 +230,7 @@ void test_chunk_create() { chunk.reset(grnxx::storage::Chunk::create(file.get())); assert(!chunk); - assert(file->resize(FILE_SIZE)); + file->resize(FILE_SIZE); chunk.reset(grnxx::storage::Chunk::create(file.get())); assert(chunk); @@ -296,7 +269,7 @@ void test_chunk_sync() { file.reset(grnxx::storage::File::create(nullptr)); assert(file); - assert(file->resize(FILE_SIZE)); + file->resize(FILE_SIZE); chunk.reset(grnxx::storage::Chunk::create(file.get())); assert(chunk); @@ -322,7 +295,7 @@ void test_chunk_flags() { file.reset(grnxx::storage::File::create(FILE_PATH)); assert(file); - assert(file->resize(1 << 20)); + file->resize(1 << 20); chunk.reset(grnxx::storage::Chunk::create(file.get())); assert(chunk); @@ -348,7 +321,7 @@ void test_chunk_address() { file.reset(grnxx::storage::File::create(nullptr)); assert(file); - assert(file->resize(10)); + file->resize(10); chunk.reset(grnxx::storage::Chunk::create(file.get())); assert(chunk); @@ -359,7 +332,7 @@ void test_chunk_address() { file.reset(grnxx::storage::File::create(FILE_PATH)); assert(file); - assert(file->resize(1 << 16)); + file->resize(1 << 16); chunk.reset(grnxx::storage::Chunk::create(file.get())); assert(chunk); @@ -389,7 +362,7 @@ void test_chunk_size() { file.reset(grnxx::storage::File::create(nullptr)); assert(file); - assert(file->resize(FILE_SIZE)); + file->resize(FILE_SIZE); chunk.reset(grnxx::storage::Chunk::create(file.get())); assert(chunk); -------------- next part -------------- HTML����������������������������...Télécharger
