Simplify serialize.h's exception handling

sipa · sipa · commit eb0b56b19017 · 2014-08-09T01:43:23.000+02:00
Remove the 'state' and 'exceptmask' from serialize.h's stream implementations,
as well as related methods.

As exceptmask always included 'failbit', and setstate was always called with
bits = failbit, all it did was immediately raise an exception. Get rid of
those variables, and replace the setstate with direct exception throwing
(which also removes some dead code).

As a result, good() is never reached after a failure (there are only 2
calls, one of which is in tests), and can just be replaced by !eof().

fail(), clear(n) and exceptions() are just never called. Delete them.
diff --git a/src/main.cpp b/src/main.cpp
@@ -3228,7 +3228,7 @@ bool LoadExternalBlockFile(FILE* fileIn, CDiskBlockPos *dbp)
             }
         }
         uint64_t nRewind = blkdat.GetPos();
-        while (blkdat.good() && !blkdat.eof()) {
+        while (!blkdat.eof()) {
             boost::this_thread::interruption_point();
 
             blkdat.SetPos(nRewind);
diff --git a/src/serialize.h b/src/serialize.h
@@ -870,8 +870,6 @@ class CDataStream
     typedef CSerializeData vector_type;
     vector_type vch;
     unsigned int nReadPos;
-    short state;
-    short exceptmask;
 public:
     int nType;
     int nVersion;
@@ -923,8 +921,6 @@ class CDataStream
         nReadPos = 0;
         nType = nTypeIn;
         nVersion = nVersionIn;
-        state = 0;
-        exceptmask = std::ios::badbit | std::ios::failbit;
     }
 
     CDataStream& operator+=(const CDataStream& b)
@@ -1047,19 +1043,7 @@ class CDataStream
     //
     // Stream subset
     //
-    void setstate(short bits, const char* psz)
-    {
-        state |= bits;
-        if (state & exceptmask)
-            throw std::ios_base::failure(psz);
-    }
-
     bool eof() const             { return size() == 0; }
-    bool fail() const            { return state & (std::ios::badbit | std::ios::failbit); }
-    bool good() const            { return !eof() && (state == 0); }
-    void clear(short n)          { state = n; }  // name conflict with vector clear()
-    short exceptions()           { return exceptmask; }
-    short exceptions(short mask) { short prev = exceptmask; exceptmask = mask; setstate(0, "CDataStream"); return prev; }
     CDataStream* rdbuf()         { return this; }
     int in_avail()               { return size(); }
 
@@ -1079,9 +1063,7 @@ class CDataStream
         {
             if (nReadPosNext > vch.size())
             {
-                setstate(std::ios::failbit, "CDataStream::read() : end of data");
-                memset(pch, 0, nSize);
-                nSize = vch.size() - nReadPos;
+                throw std::ios_base::failure("CDataStream::read() : end of data");
             }
             memcpy(pch, &vch[nReadPos], nSize);
             nReadPos = 0;
@@ -1101,7 +1083,7 @@ class CDataStream
         if (nReadPosNext >= vch.size())
         {
             if (nReadPosNext > vch.size())
-                setstate(std::ios::failbit, "CDataStream::ignore() : end of data");
+                throw std::ios_base::failure("CDataStream::ignore() : end of data");
             nReadPos = 0;
             vch.clear();
             return (*this);
@@ -1174,8 +1156,6 @@ class CAutoFile
 {
 protected:
     FILE* file;
-    short state;
-    short exceptmask;
 public:
     int nType;
     int nVersion;
@@ -1185,8 +1165,6 @@ class CAutoFile
         file = filenew;
         nType = nTypeIn;
         nVersion = nVersionIn;
-        state = 0;
-        exceptmask = std::ios::badbit | std::ios::failbit;
     }
 
     ~CAutoFile()
@@ -1213,19 +1191,6 @@ class CAutoFile
     //
     // Stream subset
     //
-    void setstate(short bits, const char* psz)
-    {
-        state |= bits;
-        if (state & exceptmask)
-            throw std::ios_base::failure(psz);
-    }
-
-    bool fail() const            { return state & (std::ios::badbit | std::ios::failbit); }
-    bool good() const            { return state == 0; }
-    void clear(short n = 0)      { state = n; }
-    short exceptions()           { return exceptmask; }
-    short exceptions(short mask) { short prev = exceptmask; exceptmask = mask; setstate(0, "CAutoFile"); return prev; }
-
     void SetType(int n)          { nType = n; }
     int GetType()                { return nType; }
     void SetVersion(int n)       { nVersion = n; }
@@ -1238,7 +1203,7 @@ class CAutoFile
         if (!file)
             throw std::ios_base::failure("CAutoFile::read : file handle is NULL");
         if (fread(pch, 1, nSize, file) != nSize)
-            setstate(std::ios::failbit, feof(file) ? "CAutoFile::read : end of file" : "CAutoFile::read : fread failed");
+            throw std::ios_base::failure(feof(file) ? "CAutoFile::read : end of file" : "CAutoFile::read : fread failed");
         return (*this);
     }
 
@@ -1247,7 +1212,7 @@ class CAutoFile
         if (!file)
             throw std::ios_base::failure("CAutoFile::write : file handle is NULL");
         if (fwrite(pch, 1, nSize, file) != nSize)
-            setstate(std::ios::failbit, "CAutoFile::write : write failed");
+            throw std::ios_base::failure("CAutoFile::write : write failed");
         return (*this);
     }
 
@@ -1292,16 +1257,7 @@ class CBufferedFile
     uint64_t nRewind;     // how many bytes we guarantee to rewind
     std::vector<char> vchBuf; // the buffer
 
-    short state;
-    short exceptmask;
-
 protected:
-    void setstate(short bits, const char *psz) {
-        state |= bits;
-        if (state & exceptmask)
-            throw std::ios_base::failure(psz);
-    }
-
     // read data from the source to fill the buffer
     bool Fill() {
         unsigned int pos = nSrcPos % vchBuf.size();
@@ -1313,8 +1269,7 @@ class CBufferedFile
             return false;
         size_t read = fread((void*)&vchBuf[pos], 1, readNow, src);
         if (read == 0) {
-            setstate(std::ios_base::failbit, feof(src) ? "CBufferedFile::Fill : end of file" : "CBufferedFile::Fill : fread failed");
-            return false;
+            throw std::ios_base::failure(feof(src) ? "CBufferedFile::Fill : end of file" : "CBufferedFile::Fill : fread failed");
         } else {
             nSrcPos += read;
             return true;
@@ -1327,12 +1282,7 @@ class CBufferedFile
 
     CBufferedFile(FILE *fileIn, uint64_t nBufSize, uint64_t nRewindIn, int nTypeIn, int nVersionIn) :
         src(fileIn), nSrcPos(0), nReadPos(0), nReadLimit((uint64_t)(-1)), nRewind(nRewindIn), vchBuf(nBufSize, 0),
-        state(0), exceptmask(std::ios_base::badbit | std::ios_base::failbit), nType(nTypeIn), nVersion(nVersionIn) {
-    }
-
-    // check whether no error occurred
-    bool good() const {
-        return state == 0;
+        nType(nTypeIn), nVersion(nVersionIn) {
     }
 
     // check whether we're at the end of the source file
@@ -1391,7 +1341,6 @@ class CBufferedFile
         nLongPos = ftell(src);
         nSrcPos = nLongPos;
         nReadPos = nLongPos;
-        state = 0;
         return true;
     }
 
diff --git a/src/test/alert_tests.cpp b/src/test/alert_tests.cpp
@@ -83,7 +83,7 @@ struct ReadAlerts
         std::vector<unsigned char> vch(alert_tests::alertTests, alert_tests::alertTests + sizeof(alert_tests::alertTests));
         CDataStream stream(vch, SER_DISK, CLIENT_VERSION);
         try {
-            while (stream.good())
+            while (!stream.eof())
             {
                 CAlert alert;
                 stream >> alert;

Original file line number	Diff line number	Diff line change
`@@ -3228,7 +3228,7 @@ bool LoadExternalBlockFile(FILE* fileIn, CDiskBlockPos *dbp)`
`3228`	`3228`	`}`
`3229`	`3229`	`}`
`3230`	`3230`	`uint64_t nRewind = blkdat.GetPos();`
`3231`		`- while (blkdat.good() && !blkdat.eof()) {`
	`3231`	`+ while (!blkdat.eof()) {`
`3232`	`3232`	`boost::this_thread::interruption_point();`
`3233`	`3233`
`3234`	`3234`	`blkdat.SetPos(nRewind);`
Original file line number	Diff line number	Diff line change
`@@ -83,7 +83,7 @@ struct ReadAlerts`
`83`	`83`	`std::vector<unsigned char> vch(alert_tests::alertTests, alert_tests::alertTests + sizeof(alert_tests::alertTests));`
`84`	`84`	`CDataStream stream(vch, SER_DISK, CLIENT_VERSION);`
`85`	`85`	`try {`
`86`		`- while (stream.good())`
	`86`	`+ while (!stream.eof())`
`87`	`87`	`{`
`88`	`88`	`CAlert alert;`
`89`	`89`	`stream >> alert;`