Fix reading of book file
authorMarco Costalba <mcostalba@gmail.com>
Sat, 7 May 2011 08:03:59 +0000 (09:03 +0100)
committerMarco Costalba <mcostalba@gmail.com>
Sat, 7 May 2011 08:51:33 +0000 (09:51 +0100)
Bug is subtle because appears only under MSVC 32 bits in
optimized version, hence was missed before.

Bug is due to the fact that evaluation order of terms of a
sum is undefined by the standard, so in get_int() we have:

return 256 * get_int<n-1>() + bookFile.get();

And if get() is evaluated before get_int() we have a corrupted
key.

The patch rewrites the code in a more natural and predictable way.

No functional change.

Signed-off-by: Marco Costalba <mcostalba@gmail.com>
src/book.cpp
src/book.h

index 8c7cd52c5ca39c482101d87f245bafcca9ef944e..26fd7c92faa930518315c46222ad31c08d326ec0 100644 (file)
@@ -502,6 +502,18 @@ int Book::find_entry(uint64_t key) {
 }
 
 
+/// Book::get_number() reads sizeof(T) chars from the file's binary byte
+/// stream and converts them in a number of type T.
+template<typename T>
+void Book::get_number(T& n) {
+
+  n = 0;
+
+  for (size_t i = 0; i < sizeof(T); i++)
+      n = (n << 8) + (T)bookFile.get();
+}
+
+
 /// Book::read_entry() takes an integer index, and returns the BookEntry
 /// at the given index in the book file.
 
@@ -514,7 +526,10 @@ BookEntry Book::read_entry(int idx) {
 
   bookFile.seekg(idx * sizeof(BookEntry), ios_base::beg);
 
-  *this >> e.key >> e.move >> e.count >> e.learn;
+  get_number(e.key);
+  get_number(e.move);
+  get_number(e.count);
+  get_number(e.learn);
 
   if (!bookFile.good())
   {
index ed68210f8daf83fb14f832b6f0aa709001fe73b2..8299b0315eaccfd40972eea436fe7fb855819b56 100644 (file)
@@ -48,12 +48,7 @@ public:
   const std::string name() const { return bookName; }
 
 private:
-  // read n chars from the file stream and converts them in an
-  // integer number. Integers are stored with highest byte first.
-  template<int n> uint64_t get_int();
-
-  template<typename T>
-  Book& operator>>(T& n) { n = (T)get_int<sizeof(T)>(); return *this; }
+  template<typename T> void get_number(T& n);
 
   BookEntry read_entry(int idx);
   int find_entry(uint64_t key);
@@ -64,8 +59,4 @@ private:
   RKISS RKiss;
 };
 
-// Yes, we indulge a bit here ;-)
-template<int n> inline uint64_t Book::get_int() { return 256 * get_int<n-1>() + bookFile.get(); }
-template<> inline uint64_t Book::get_int<1>() { return bookFile.get(); }
-
 #endif // !defined(BOOK_H_INCLUDED)