[Groonga-commit] groonga/grnxx at 1b1ef7c [master] Update grnxx::storage::Chunk to use uint64_t for offset and size.

Back to archive index

susumu.yata null+****@clear*****
Wed May 15 16:08:59 JST 2013


susumu.yata	2013-05-15 16:08:59 +0900 (Wed, 15 May 2013)

  New Revision: 1b1ef7c71fa1f384708445213d9e46323a32a7ff
  https://github.com/groonga/grnxx/commit/1b1ef7c71fa1f384708445213d9e46323a32a7ff

  Message:
    Update grnxx::storage::Chunk to use uint64_t for offset and size.

  Modified files:
    lib/grnxx/storage/chunk-posix.cpp
    lib/grnxx/storage/chunk-posix.hpp
    lib/grnxx/storage/chunk-windows.cpp
    lib/grnxx/storage/chunk-windows.hpp
    lib/grnxx/storage/chunk.cpp
    lib/grnxx/storage/chunk.hpp
    test/test_storage.cpp

  Modified: lib/grnxx/storage/chunk-posix.cpp (+24 -15)
===================================================================
--- lib/grnxx/storage/chunk-posix.cpp    2013-05-15 15:31:03 +0900 (6c6a1f4)
+++ lib/grnxx/storage/chunk-posix.cpp    2013-05-15 16:08:59 +0900 (5d516b4)
@@ -22,6 +22,7 @@
 #include <sys/mman.h>
 
 #include <cerrno>
+#include <limits>
 #include <memory>
 #include <new>
 
@@ -51,7 +52,7 @@ ChunkImpl::~ChunkImpl() {
   }
 }
 
-ChunkImpl *ChunkImpl::create(File *file, int64_t offset, int64_t size,
+ChunkImpl *ChunkImpl::create(File *file, uint64_t offset, uint64_t size,
                              ChunkFlags flags) {
   std::unique_ptr<ChunkImpl> chunk(new (std::nothrow) ChunkImpl);
   if (!chunk) {
@@ -70,20 +71,23 @@ ChunkImpl *ChunkImpl::create(File *file, int64_t offset, int64_t size,
   return chunk.release();
 }
 
-bool ChunkImpl::sync(int64_t offset, int64_t size) {
+bool ChunkImpl::sync(uint64_t offset, uint64_t size) {
   if ((flags_ & CHUNK_ANONYMOUS) || (flags_ & CHUNK_READ_ONLY)) {
     GRNXX_WARNING() << "invalid operation: flags = " << flags_;
     return false;
   }
-  if ((offset < 0) || (offset > size_) || (size > size_) ||
-      ((size >= 0) && (size > (size_ - offset)))) {
+  if ((offset > size_) || (size > size_) || (size > (size_ - offset))) {
     GRNXX_ERROR() << "invalid argument: offset = " << offset
                   << ", size = " << size << ", chunk_size = " << size_;
     return false;
   }
-  if (size < 0) {
+  if (size == 0) {
     size = size_ - offset;
   }
+  if (size > std::numeric_limits<size_t>::max()) {
+    GRNXX_ERROR() << "invalid argument: size = " << size;
+    return false;
+  }
   if (size > 0) {
     if (::msync(static_cast<char *>(address_) + offset, size, MS_SYNC) != 0) {
       GRNXX_ERROR() << "failed to sync chunk: offset = " << offset
@@ -102,23 +106,28 @@ void *ChunkImpl::address() const {
   return address_;
 }
 
-int64_t ChunkImpl::size() const {
+uint64_t ChunkImpl::size() const {
   return size_;
 }
 
-bool ChunkImpl::create_file_backed_chunk(File *file, int64_t offset,
-                                         int64_t size, ChunkFlags flags) {
-  const int64_t file_size = file->size();
-  if ((offset < 0) || (offset >= file_size) ||
-      (size == 0) || (size > file_size) ||
-      ((size > 0) && (size > (file_size - offset)))) {
+bool ChunkImpl::create_file_backed_chunk(File *file, uint64_t offset,
+                                         uint64_t size, ChunkFlags flags) {
+  const uint64_t file_size = file->size();
+  if ((offset >= file_size) || (size > file_size) ||
+      (size > (file_size - offset))) {
     GRNXX_ERROR() << "invalid argument: offset = " << offset
                   << ", size = " << size << ", file_size = " << file_size;
     return false;
   }
-  if (size < 0) {
+  if (size == 0) {
     size = file_size - offset;
   }
+  if ((offset > static_cast<uint64_t>(std::numeric_limits<off_t>::max())) ||
+      (size > std::numeric_limits<size_t>::max())) {
+    GRNXX_ERROR() << "invalid argument: offset = " << offset
+                  << ", size = " << size;
+    return false;
+  }
   if (file->flags() & FILE_READ_ONLY) {
     flags_ |= CHUNK_READ_ONLY;
   }
@@ -142,8 +151,8 @@ bool ChunkImpl::create_file_backed_chunk(File *file, int64_t offset,
   return true;
 }
 
-bool ChunkImpl::create_anonymous_chunk(int64_t size, ChunkFlags flags) {
-  if (size <= 0) {
+bool ChunkImpl::create_anonymous_chunk(uint64_t size, ChunkFlags flags) {
+  if ((size == 0) || (size > std::numeric_limits<size_t>::max())) {
     GRNXX_ERROR() << "invalid argument: size = " << size;
     return false;
   }

  Modified: lib/grnxx/storage/chunk-posix.hpp (+6 -6)
===================================================================
--- lib/grnxx/storage/chunk-posix.hpp    2013-05-15 15:31:03 +0900 (0e842f2)
+++ lib/grnxx/storage/chunk-posix.hpp    2013-05-15 16:08:59 +0900 (8cf872e)
@@ -32,23 +32,23 @@ class ChunkImpl : public Chunk {
   ChunkImpl();
   ~ChunkImpl();
 
-  static ChunkImpl *create(File *file, int64_t offset, int64_t size,
+  static ChunkImpl *create(File *file, uint64_t offset, uint64_t size,
                            ChunkFlags flags);
 
-  bool sync(int64_t offset, int64_t size);
+  bool sync(uint64_t offset, uint64_t size);
 
   ChunkFlags flags() const;
   void *address() const;
-  int64_t size() const;
+  uint64_t size() const;
 
  private:
   ChunkFlags flags_;
   void *address_;
-  int64_t size_;
+  uint64_t size_;
 
-  bool create_file_backed_chunk(File *file, int64_t offset, int64_t size,
+  bool create_file_backed_chunk(File *file, uint64_t offset, uint64_t size,
                                 ChunkFlags flags);
-  bool create_anonymous_chunk(int64_t size, ChunkFlags flags);
+  bool create_anonymous_chunk(uint64_t size, ChunkFlags flags);
 };
 
 }  // namespace storage

  Modified: lib/grnxx/storage/chunk-windows.cpp (+29 -28)
===================================================================
--- lib/grnxx/storage/chunk-windows.cpp    2013-05-15 15:31:03 +0900 (74d5c37)
+++ lib/grnxx/storage/chunk-windows.cpp    2013-05-15 16:08:59 +0900 (7c22510)
@@ -36,9 +36,9 @@ ChunkImpl::ChunkImpl()
 
 ChunkImpl::~ChunkImpl() {
   if (address_) {
-    if (!::UnmapChunkOfFile(address_)) {
+    if (!::UnmapViewOfFile(address_)) {
       GRNXX_ERROR() << "failed to unmap chunk"
-                    << ": '::UnmapChunkOfFile' " << Error(::GetLastError());
+                    << ": '::UnmapViewOfFile' " << Error(::GetLastError());
     }
   }
   if (handle_) {
@@ -49,7 +49,7 @@ ChunkImpl::~ChunkImpl() {
   }
 }
 
-ChunkImpl *ChunkImpl::create(File *file, int64_t offset, int64_t size,
+ChunkImpl *ChunkImpl::create(File *file, uint64_t offset, uint64_t size,
                              ChunkFlags flags) {
   std::unique_ptr<ChunkImpl> chunk(new (std::nothrow) ChunkImpl);
   if (!chunk) {
@@ -68,44 +68,45 @@ ChunkImpl *ChunkImpl::create(File *file, int64_t offset, int64_t size,
   return chunk.release();
 }
 
-bool ChunkImpl::sync(int64_t offset, int64_t size) {
+bool ChunkImpl::sync(uint64_t offset, uint64_t size) {
   if ((flags_ & CHUNK_ANONYMOUS) || (flags_ & CHUNK_READ_ONLY)) {
     GRNXX_WARNING() << "invalid operation: flags = " << flags_;
     return false;
   }
-  if ((offset < 0) || (offset > size_) || (size > size_) ||
-      ((size >= 0) && (size > (size_ - offset)))) {
+  if ((offset > size_) || (size > size_) || (size > (size_ - offset))) {
     GRNXX_ERROR() << "invalid argument: offset = " << offset
                   << ", size = " << size << ", chunk_size = " << size_;
     return false;
   }
-  if (size < 0) {
-    size = size_ - offset;
+  if (size > std::numeric_limits<SIZE_T>::max()) {
+    GRNXX_ERROR() << "invalid argument: size = " << size;
+    return false;
   }
-  if (size > 0) {
-    if (!::FlushChunkOfFile(static_cast<char *>(address_) + offset, size)) {
-      GRNXX_ERROR() << "failed to sync chunk: offset = " << offset
-                    << ", size = " << size
-                    << ": '::FlushChunkOfFile' " << Error(::GetLastError());
-      return false;
-    }
+  if (!::FlushViewOfFile(static_cast<char *>(address_) + offset, size)) {
+    GRNXX_ERROR() << "failed to sync chunk: offset = " << offset
+                  << ", size = " << size
+                  << ": '::FlushViewOfFile' " << Error(::GetLastError());
+    return false;
   }
   return true;
 }
 
-bool ChunkImpl::create_file_backed_chunk(File *file, int64_t offset,
-                                         int64_t size, ChunkFlags flags) {
-  const int64_t file_size = file->size();
-  if ((offset < 0) || (offset >= file_size) ||
-      (size == 0) || (size > file_size) ||
-      ((size > 0) && (size > (file_size - offset)))) {
+bool ChunkImpl::create_file_backed_chunk(File *file, uint64_t offset,
+                                         uint64_t size, ChunkFlags flags) {
+  const uint64_t file_size = file->size();
+  if ((offset >= file_size) || (size > file_size) ||
+      (size > (file_size - offset))) {
     GRNXX_ERROR() << "invalid argument: offset = " << offset
                   << ", size = " << size << ", file_size = " << file_size;
     return false;
   }
-  if (size < 0) {
+  if (size == 0) {
     size = file_size - offset;
   }
+  if (size > std::numeric_limits<SIZE_T>::max()) {
+    GRNXX_ERROR() << "invalid argument: size = " << size;
+    return false;
+  }
   if (file->flags() & FILE_READ_ONLY) {
     flags_ |= CHUNK_READ_ONLY;
   }
@@ -131,21 +132,21 @@ bool ChunkImpl::create_file_backed_chunk(File *file, int64_t offset,
   }
   const DWORD offset_high = static_cast<DWORD>(offset >> 32);
   const DWORD offset_low = static_cast<DWORD>(offset);
-  address_ = ::MapChunkOfFile(handle_, desired_access, offset_high, offset_low,
+  address_ = ::MapViewOfFile(handle_, desired_access, offset_high, offset_low,
                              static_cast<SIZE_T>(size));
   if (!address_) {
     GRNXX_ERROR() << "failed to map chunk: "
                   << "file_path = " << file->path()
                   << ", file_size = " << file_size << ", offset = " << offset
                   << ", size = " << size << ", flags = " << flags
-                  << ": '::MapChunkOfFile' " << Error(::GetLastError());
+                  << ": '::MapViewOfFile' " << Error(::GetLastError());
     return false;
   }
   return true;
 }
 
-bool ChunkImpl::create_anonymous_chunk(int64_t size, ChunkFlags flags) {
-  if (size <= 0) {
+bool ChunkImpl::create_anonymous_chunk(uint64_t size, ChunkFlags flags) {
+  if (size == 0) {
     GRNXX_ERROR() << "invalid argument: size = " << size;
     return false;
   }
@@ -161,11 +162,11 @@ bool ChunkImpl::create_anonymous_chunk(int64_t size, ChunkFlags flags) {
                   << ": '::CreateFileMapping' " << Error(::GetLastError());
     return false;
   }
-  address_ = ::MapChunkOfFile(handle_, FILE_MAP_WRITE, 0, 0, 0);
+  address_ = ::MapViewOfFile(handle_, FILE_MAP_WRITE, 0, 0, 0);
   if (!address_) {
     GRNXX_ERROR() << "failed to map anonymous chunk: "
                   << "size = " << size << ", flags = " << flags
-                  << ": '::MapChunkOfFile' " << Error(::GetLastError());
+                  << ": '::MapViewOfFile' " << Error(::GetLastError());
     return false;
   }
   return true;

  Modified: lib/grnxx/storage/chunk-windows.hpp (+5 -5)
===================================================================
--- lib/grnxx/storage/chunk-windows.hpp    2013-05-15 15:31:03 +0900 (55c0a3f)
+++ lib/grnxx/storage/chunk-windows.hpp    2013-05-15 16:08:59 +0900 (4c543e2)
@@ -39,14 +39,14 @@ class ChunkImpl : public Chunk {
   ChunkImpl();
   ~ChunkImpl();
 
-  static ChunkImpl *create(File *file, int64_t offset, int64_t size,
+  static ChunkImpl *create(File *file, uint64_t offset, uint64_t size,
                            ChunkFlags flags);
 
-  bool sync(int64_t offset, int64_t size);
+  bool sync(uint64_t offset, uint64_t size);
 
   ChunkFlags flags() const;
   void *address() const;
-  int64_t size() const;
+  uint64_t size() const;
 
  private:
   ChunkFlags flags_;
@@ -54,9 +54,9 @@ class ChunkImpl : public Chunk {
   void *address_;
   uint64_t size_;
 
-  bool create_file_backed_chunk(File *file, int64_t offset, int64_t size,
+  bool create_file_backed_chunk(File *file, uint64_t offset, uint64_t size,
                                 ChunkFlags flags);
-  bool create_anonymous_chunk(int64_t size, ChunkFlags flags);
+  bool create_anonymous_chunk(uint64_t size, ChunkFlags flags);
 };
 
 }  // namespace storage

  Modified: lib/grnxx/storage/chunk.cpp (+1 -1)
===================================================================
--- lib/grnxx/storage/chunk.cpp    2013-05-15 15:31:03 +0900 (ce27354)
+++ lib/grnxx/storage/chunk.cpp    2013-05-15 16:08:59 +0900 (40c3331)
@@ -48,7 +48,7 @@ StringBuilder &operator<<(StringBuilder &builder, ChunkFlags flags) {
 Chunk::Chunk() {}
 Chunk::~Chunk() {}
 
-Chunk *Chunk::create(File *file, int64_t offset, int64_t size,
+Chunk *Chunk::create(File *file, uint64_t offset, uint64_t size,
                      ChunkFlags flags) {
   return ChunkImpl::create(file, offset, size, flags);
 }

  Modified: lib/grnxx/storage/chunk.hpp (+3 -5)
===================================================================
--- lib/grnxx/storage/chunk.hpp    2013-05-15 15:31:03 +0900 (4b3be92)
+++ lib/grnxx/storage/chunk.hpp    2013-05-15 16:08:59 +0900 (9dce8f1)
@@ -55,20 +55,18 @@ class Chunk {
   // Create a file-backed memory mapping on "file" if "file" != nullptr, or
   // create an anonymous memory mapping.
   // The available flag is CHUNK_HUGE_TLB.
-  static Chunk *create(File *file,
-                       int64_t offset = 0,
-                       int64_t size = -1,
+  static Chunk *create(File *file, uint64_t offset = 0, uint64_t size = 0,
                        ChunkFlags flags = CHUNK_DEFAULT);
 
   // Flush modified pages.
-  virtual bool sync(int64_t offset = 0, int64_t size = -1) = 0;
+  virtual bool sync(uint64_t offset = 0, uint64_t size = 0) = 0;
 
   // Return the enabled flags.
   virtual ChunkFlags flags() const = 0;
   // Return the starting address.
   virtual void *address() const = 0;
   // Return the size.
-  virtual int64_t size() const = 0;
+  virtual uint64_t size() const = 0;
 };
 
 }  // namespace storage

  Modified: test/test_storage.cpp (+4 -7)
===================================================================
--- test/test_storage.cpp    2013-05-15 15:31:03 +0900 (9412d43)
+++ test/test_storage.cpp    2013-05-15 16:08:59 +0900 (3f2413a)
@@ -16,15 +16,16 @@
   Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
 */
 #include <cassert>
-#include <sstream>
+#include <memory>
 #include <random>
 #include <unordered_set>
 
+#include "grnxx/logger.hpp"
 #include "grnxx/storage.hpp"
 #include "grnxx/storage/file.hpp"
 #include "grnxx/storage/path.hpp"
 #include "grnxx/storage/chunk.hpp"
-#include "grnxx/logger.hpp"
+#include "grnxx/types.hpp"
 
 namespace {
 
@@ -254,7 +255,7 @@ void test_chunk_create() {
   assert(chunk);
   chunk.reset(grnxx::storage::Chunk::create(file.get(), 0));
   assert(chunk);
-  chunk.reset(grnxx::storage::Chunk::create(file.get(), 0, -1));
+  chunk.reset(grnxx::storage::Chunk::create(file.get(), 0, 0));
   assert(chunk);
   chunk.reset(grnxx::storage::Chunk::create(file.get(), 0, file->size()));
   assert(chunk);
@@ -265,8 +266,6 @@ void test_chunk_create() {
   assert(!chunk);
   chunk.reset(grnxx::storage::Chunk::create(file.get(), file->size() + 1));
   assert(!chunk);
-  chunk.reset(grnxx::storage::Chunk::create(file.get(), 0, 0));
-  assert(!chunk);
   chunk.reset(grnxx::storage::Chunk::create(file.get(), 0, file->size() + 1));
   assert(!chunk);
   chunk.reset(grnxx::storage::Chunk::create(file.get(), file->size() / 2,
@@ -294,11 +293,9 @@ void test_chunk_sync() {
   assert(chunk);
   assert(chunk->sync());
   assert(chunk->sync(0));
-  assert(chunk->sync(0, -1));
   assert(chunk->sync(0, 0));
   assert(chunk->sync(0, file->size()));
 
-  assert(!chunk->sync(-1));
   assert(!chunk->sync(file->size() + 1));
   assert(!chunk->sync(0, file->size() + 1));
   assert(!chunk->sync(file->size() / 2, file->size()));
-------------- next part --------------
HTML����������������������������...
Download 



More information about the Groonga-commit mailing list
Back to archive index