From 2535dc13403acba764696587b920dee9493ddddd Mon Sep 17 00:00:00 2001 From: Marco Costalba Date: Sat, 7 May 2011 09:03:59 +0100 Subject: [PATCH] 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() + 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 --- src/book.cpp | 17 ++++++++++++++++- src/book.h | 11 +---------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/book.cpp b/src/book.cpp index 8c7cd52c..26fd7c92 100644 --- 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 +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()) { diff --git a/src/book.h b/src/book.h index ed68210f..8299b031 100644 --- a/src/book.h +++ b/src/book.h @@ -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 uint64_t get_int(); - - template - Book& operator>>(T& n) { n = (T)get_int(); return *this; } + template 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 inline uint64_t Book::get_int() { return 256 * get_int() + bookFile.get(); } -template<> inline uint64_t Book::get_int<1>() { return bookFile.get(); } - #endif // !defined(BOOK_H_INCLUDED) -- 2.39.2