Fix reading of book file

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>

diff --git a/src/book.cpp b/src/book.cpp
index 8c7cd52c5ca39c482101d87f245bafcca9ef944e..26fd7c92faa930518315c46222ad31c08d326ec0 100644 (file)
--- a/src/book.cpp
+++ b/src/book.cpp
@@ -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.
 
 /// 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);
 
 
   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())
   {
 
   if (!bookFile.good())
   {

diff --git a/src/book.h b/src/book.h
index ed68210f8daf83fb14f832b6f0aa709001fe73b2..8299b0315eaccfd40972eea436fe7fb855819b56 100644 (file)
--- a/src/book.h
+++ b/src/book.h
@@ -48,12 +48,7 @@ public:
   const std::string name() const { return bookName; }
 
 private:
   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);
 
   BookEntry read_entry(int idx);
   int find_entry(uint64_t key);


@@ -64,8 +59,4 @@ private:
   RKISS RKiss;
 };
 
   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)
 #endif // !defined(BOOK_H_INCLUDED)