From 6324deb7d0b08daf1bddbbf19a6e745eabf7e9fa Mon Sep 17 00:00:00 2001 From: janwas Date: Sun, 15 Jun 2008 17:24:32 +0000 Subject: [PATCH] fix to prevent further issues due to invalid XMB files (c.f. matt's "crash upon combat" bug report, http://www.wildfiregames.com/forum/index.php?showtopic=11703). as per philip's excellent suggestion, they are now only tagged with a header iff the file contents are valid. details: - add a new magic string "XMBu" to indicate an unfinished XMB file; - header magic fields are now exclusively strings to avoid type punning; - XMBFile::Initialize now has a return value which is checked by ReadXMBFile; - the XML loader re-creates the XMB if that fails. This was SVN commit r6016. --- source/ps/XML/XeroXMB.cpp | 18 ++++++++++++------ source/ps/XML/XeroXMB.h | 7 +++++-- source/ps/XML/Xeromyces.cpp | 15 ++++++++++----- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/source/ps/XML/XeroXMB.cpp b/source/ps/XML/XeroXMB.cpp index 2086b85c6e..8e5bc96a78 100644 --- a/source/ps/XML/XeroXMB.cpp +++ b/source/ps/XML/XeroXMB.cpp @@ -2,20 +2,25 @@ #include "Xeromyces.h" +#include "lib/byte_order.h" // FOURCC_LE #include "ps/utf16string.h" - - -const u32 HeaderMagic = 0x30424D58; // = "XMB0" (little-endian) +// external linkage (also used by Xeromyces.cpp) const char* HeaderMagicStr = "XMB0"; +const char* UnfinishedHeaderMagicStr = "XMBu"; // Warning: May contain traces of pointer abuse -void XMBFile::Initialise(const char* FileData) +bool XMBFile::Initialise(const char* FileData) { m_Pointer = FileData; - u32 Header = *(u32 *)m_Pointer; m_Pointer += 4; - debug_assert(Header == HeaderMagic && "Invalid XMB header!"); + char Header[5]; + strncpy_s(Header, 5, m_Pointer, 4); + m_Pointer += 4; + // (c.f. @return documentation of this function) + if(!strcmp(Header, UnfinishedHeaderMagicStr)) + return false; + debug_assert(!strcmp(Header, HeaderMagicStr) && "Invalid XMB header!"); int i; @@ -47,6 +52,7 @@ void XMBFile::Initialise(const char* FileData) m_Pointer += 4 + *(int*)m_Pointer; // skip over the string #endif + return true; // success } std::string XMBFile::ReadZStrA() diff --git a/source/ps/XML/XeroXMB.h b/source/ps/XML/XeroXMB.h index 0d54239d40..8f53644313 100644 --- a/source/ps/XML/XeroXMB.h +++ b/source/ps/XML/XeroXMB.h @@ -94,8 +94,8 @@ XMB_Text { #endif // File headers, to make sure it doesn't try loading anything other than an XMB -extern const u32 HeaderMagic; extern const char* HeaderMagicStr; +extern const char* UnfinishedHeaderMagicStr; class XMBElement; class XMBElementList; @@ -111,7 +111,10 @@ public: // Initialise from the contents of an XMB file. // FileData must remain allocated and unchanged while // the XMBFile is being used. - void Initialise(const char* FileData); + // @return indication of success; main cause for failure is attempting to + // load a partially valid XMB file (e.g. if the game was interrupted + // while writing it), which we detect by checking the magic string. + bool Initialise(const char* FileData); // Returns the root element XMBElement GetRoot() const; diff --git a/source/ps/XML/Xeromyces.cpp b/source/ps/XML/Xeromyces.cpp index 80d2316ea8..b83243bbc7 100644 --- a/source/ps/XML/Xeromyces.cpp +++ b/source/ps/XML/Xeromyces.cpp @@ -177,8 +177,8 @@ PSRETURN CXeromyces::Load(const VfsPath& filename) { if (ReadXMBFile(xmbPath)) return PSRETURN_OK; - else - return PSRETURN_Xeromyces_XMLOpenFailed; + // (no longer return PSRETURN_Xeromyces_XMLOpenFailed here because + // failure legitimately happens due to partially-written XMB files.) } @@ -243,7 +243,8 @@ PSRETURN CXeromyces::Load(const VfsPath& filename) XMBBuffer = writeBuffer.Data(); // add a reference // Set up the XMBFile - Initialise((const char*)XMBBuffer.get()); + const bool ok = Initialise((const char*)XMBBuffer.get()); + debug_assert(ok); return PSRETURN_OK; } @@ -256,7 +257,8 @@ bool CXeromyces::ReadXMBFile(const VfsPath& filename) debug_assert(size >= 42); // else: invalid XMB file size. (42 bytes is the smallest possible XMB. (Well, maybe not quite, but it's a nice number.)) // Set up the XMBFile - Initialise((const char*)XMBBuffer.get()); + if(!Initialise((const char*)XMBBuffer.get())) + return false; return true; } @@ -349,7 +351,7 @@ void XeroHandler::characters(const XMLCh* const chars, const unsigned int UNUSED void XeroHandler::CreateXMB() { // Header - writeBuffer.Append((void*)HeaderMagicStr, 4); + writeBuffer.Append(UnfinishedHeaderMagicStr, 4); std::set::iterator it; int i; @@ -385,6 +387,9 @@ void XeroHandler::CreateXMB() delete Root; Root = NULL; + + // file is now valid, so insert correct magic string + writeBuffer.Overwrite(HeaderMagicStr, 4, 0); } // Writes a whole element (recursively if it has children) into the buffer,