diff --git a/Global.h b/Global.h index d070882d7..b250e1571 100644 --- a/Global.h +++ b/Global.h @@ -349,6 +349,9 @@ namespace range = boost::range; template char (&_ArrayCountObj(const T (&)[N]))[N]; #define ARRAY_COUNT(arr) (sizeof(_ArrayCountObj(arr))) + +#define THROW_FORMAT(message, formatting_elems) throw std::runtime_error(boost::str(boost::format(message) % formatting_elems)) + //XXX pls dont - 'debug macros' are usually more trouble than it's worth #define HANDLE_EXCEPTION \ catch (const std::exception& e) { \ diff --git a/client/CPreGame.cpp b/client/CPreGame.cpp index 0b08f2bd7..f1cccddb5 100644 --- a/client/CPreGame.cpp +++ b/client/CPreGame.cpp @@ -1063,27 +1063,30 @@ void SelectionTab::parseGames(std::vector &files, bool multi) { for(int i=0; i> sign; - if(std::memcmp(sign,"VCMISVG",7)) + try { - tlog1 << files[i].name << " is not a correct savefile!" << std::endl; - continue; + CLoadFile lf(files[i].name); + + ui8 sign[8]; + lf >> sign; + if(std::memcmp(sign,"VCMISVG",7)) + throw std::runtime_error("not a correct savefile!"); + + allItems[i].mapHeader = new CMapHeader(); + lf >> *(allItems[i].mapHeader) >> allItems[i].scenarioOpts; + allItems[i].filename = files[i].name; + allItems[i].countPlayers(); + allItems[i].date = std::asctime(std::localtime(&files[i].date)); + + if((allItems[i].actualHumanPlayers > 1) != multi) //if multi mode then only multi games, otherwise single + { + vstd::clear_pointer(allItems[i].mapHeader); + } } - allItems[i].mapHeader = new CMapHeader(); - lf >> *(allItems[i].mapHeader) >> allItems[i].scenarioOpts; - allItems[i].filename = files[i].name; - allItems[i].countPlayers(); - allItems[i].date = std::asctime(std::localtime(&files[i].date)); - - if((allItems[i].actualHumanPlayers > 1) != multi) //if multi mode then only multi games, otherwise single + catch(std::exception &e) { - delete allItems[i].mapHeader; - allItems[i].mapHeader = NULL; + vstd::clear_pointer(allItems[i].mapHeader); + tlog3 << "Failed to process " << files[i].name <<": " << e.what() << std::endl; } } } diff --git a/lib/Connection.cpp b/lib/Connection.cpp index 40e22291d..ec6caeb4d 100644 --- a/lib/Connection.cpp +++ b/lib/Connection.cpp @@ -302,54 +302,56 @@ CLoadFile::~CLoadFile() int CLoadFile::read( const void * data, unsigned size ) { - char *bytePtr = (char *)data; - sfile->read(bytePtr, size); + sfile->read((char *)data,size); return size; } void CLoadFile::openNextFile(const std::string &fname, int minimalVersion) { - fName = fname; - sfile = make_unique(fname.c_str(),std::ios::binary); - if(!(*sfile)) - { - tlog1 << "Error: cannot open to read " << fname << std::endl; - sfile.release(); - } - else + assert(!reverseEndianess); + assert(minimalVersion <= version); + + try { + fName = fname; + sfile = make_unique(fname, std::ios::binary); + sfile->exceptions(std::ifstream::failbit | std::ifstream::badbit); //we throw a lot anyway + + if(!(*sfile)) + THROW_FORMAT("Error: cannot open to read %s!", fname); + + //we can read char buffer[4]; sfile->read(buffer, 4); - if(std::memcmp(buffer,"VCMI",4)) - { - tlog1 << "Error: not a VCMI file! ( " << fname << " )\n"; - sfile.release(); - return; - } + THROW_FORMAT("Error: not a VCMI file(%s)!", fname); - *this >> myVersion; - if(myVersion < minimalVersion) + *this >> fileVersion; + if(fileVersion < minimalVersion) + THROW_FORMAT("Error: too old file format (%s)!", fname); + + if(fileVersion > version) { - tlog1 << "Error: Too old file format! (file " << fname << " )\n"; - sfile.release(); - } - if(myVersion > version) - { - auto versionptr = (char*)&myVersion; + tlog3 << boost::format("Warning format version mismatch: found %d when current is %d! (file %s)\n") % fileVersion % version % fname; + + auto versionptr = (char*)&fileVersion; std::reverse(versionptr, versionptr + 4); - if(myVersion == version) + tlog3 << "Version number reversed is " << fileVersion << ", checking...\n"; + + if(fileVersion == version) { + tlog3 << fname << " seems to have different endianess! Entering reversing mode.\n"; reverseEndianess = true; - tlog3 << fname << " seems to have different endianess!\n"; } else - { - tlog1 << "Error: Too new file format! (file " << fname << " )\n"; - sfile.release(); - } + THROW_FORMAT("Error: too new file format (%s)!", fname); } } + catch(...) + { + clear(); //if anything went wrong, we delete file and rethrow + throw; + } } void CLoadFile::reportState(CLogger &out) @@ -361,6 +363,13 @@ void CLoadFile::reportState(CLogger &out) } } +void CLoadFile::clear() +{ + sfile = nullptr; + fName.clear(); + fileVersion = 0; +} + CTypeList::CTypeList() { registerTypes(*this); diff --git a/lib/Connection.h b/lib/Connection.h index 6c64b22f9..615747c56 100644 --- a/lib/Connection.h +++ b/lib/Connection.h @@ -686,7 +686,7 @@ template class DLL_LINKAGE CISer : public CLoaderBase public: bool saving; std::map loaders; // typeID => CPointerSaver - ui32 myVersion; + ui32 fileVersion; bool reverseEndianess; //if source has different endianess than us, we reverse bytes std::map loadedPointers; @@ -695,7 +695,7 @@ public: CISer() { saving = false; - myVersion = version; + fileVersion = 0; smartPointerSerialization = true; reverseEndianess = false; } @@ -760,12 +760,18 @@ public: template void loadPrimitive(T &data) { - char * dataPtr = (char*)&data; - unsigned length = sizeof(data); - - this->This()->read(dataPtr,length); - if(reverseEndianess) - std::reverse(dataPtr, dataPtr + length); + if(0) //for testing #989 + { + this->This()->read(&data,sizeof(data)); + } + else + { + unsigned length = sizeof(data); + char* dataPtr = (char*)&data; + this->This()->read(dataPtr,length); + if(reverseEndianess) + std::reverse(dataPtr, dataPtr + length); + } } template @@ -774,7 +780,7 @@ public: ////that const cast is evil because it allows to implicitly overwrite const objects when deserializing typedef typename boost::remove_const::type nonConstT; nonConstT &hlp = const_cast(data); - hlp.serialize(*this,myVersion); + hlp.serialize(*this,fileVersion); //data.serialize(*this,myVersion); } @@ -1018,11 +1024,13 @@ public: std::string fName; unique_ptr sfile; - CLoadFile(const std::string &fname, int minimalVersion = version); + CLoadFile(const std::string &fname, int minimalVersion = version); //throws! ~CLoadFile(); int read(const void * data, unsigned size); - void openNextFile(const std::string &fname, int minimalVersion); + void openNextFile(const std::string &fname, int minimalVersion); //throws! + + void clear(); void reportState(CLogger &out); };