From 874e530d84f5d2acb020f91007c0b8bc408498dc Mon Sep 17 00:00:00 2001 From: "v.audacity" Date: Thu, 29 Jul 2010 03:54:54 +0000 Subject: [PATCH] more progress on bug 113 better handling of error conditions in PCMAliasBlockFile::BuildFromXML so that DirManager::ProjectFSCK can report cases of missing PCMAliasBlockFile files on opening projects where missing files were not corrected --- src/DirManager.cpp | 7 +-- src/blockfile/PCMAliasBlockFile.cpp | 70 ++++++++++++----------------- src/xml/XMLTagHandler.cpp | 36 ++++++++------- src/xml/XMLTagHandler.h | 4 +- 4 files changed, 54 insertions(+), 63 deletions(-) diff --git a/src/DirManager.cpp b/src/DirManager.cpp index f580a80e6..de671c26d 100644 --- a/src/DirManager.cpp +++ b/src/DirManager.cpp @@ -548,8 +548,8 @@ wxFileName DirManager::MakeBlockFilePath(wxString value){ return dir; } -bool DirManager::AssignFile(wxFileName &fileName, - wxString value, +bool DirManager::AssignFile(wxFileName &fileName, + wxString value, bool diskcheck){ wxFileName dir=MakeBlockFilePath(value); @@ -1552,7 +1552,7 @@ int DirManager::ProjectFSCK(bool forceerror, bool silentlycorrect, bool bIgnoreN // silence the blockfiles by yanking the filename //vvvvv But this causes Check Dependencies to show "MISSING" with no filename. //vvvvv Replace with actual SilentBlockFile, as that's what the user commanded. - //vvvvv Call RemoveDependencies from Dependencies.cpp instead? + //vvvvv Call RemoveDependencies (from Dependencies.*) instead? wxFileName dummy; dummy.Clear(); b->ChangeAliasedFile(dummy); @@ -1560,6 +1560,7 @@ int DirManager::ProjectFSCK(bool forceerror, bool silentlycorrect, bool bIgnoreN ret |= FSCKstatus_CHANGED; }else if(action==1){ // silence the log for this session + //vvvvv Should do more than that...! b->SilenceAliasLog(); } i++; diff --git a/src/blockfile/PCMAliasBlockFile.cpp b/src/blockfile/PCMAliasBlockFile.cpp index 01d58b20a..fe8ce63a7 100644 --- a/src/blockfile/PCMAliasBlockFile.cpp +++ b/src/blockfile/PCMAliasBlockFile.cpp @@ -182,7 +182,6 @@ void PCMAliasBlockFile::SaveXML(XMLWriter &xmlFile) BlockFile *PCMAliasBlockFile::BuildFromXML(DirManager &dm, const wxChar **attrs) { - bool bMissingAliasFile = false; wxFileName summaryFileName; wxFileName aliasFileName; int aliasStart=0, aliasLen=0, aliasChannel=0; @@ -201,15 +200,13 @@ BlockFile *PCMAliasBlockFile::BuildFromXML(DirManager &dm, const wxChar **attrs) if( !wxStricmp(attr, wxT("summaryfile")) ) { // Can't use XMLValueChecker::IsGoodFileName here, but do part of its test. - if (!XMLValueChecker::IsGoodFileString(strValue)) - return NULL; - - #ifdef _WIN32 - if (strValue.Length() + 1 + dm.GetProjectDataDir().Length() > MAX_PATH) - return NULL; - #endif - - dm.AssignFile(summaryFileName,value,FALSE); + if (XMLValueChecker::IsGoodFileString(strValue)) + { + #ifdef _WIN32 + if (strValue.Length() + 1 + dm.GetProjectDataDir().Length() <= MAX_PATH) + #endif + dm.AssignFile(summaryFileName, strValue, false); + } } else if( !wxStricmp(attr, wxT("aliasfile")) ) { @@ -218,26 +215,38 @@ BlockFile *PCMAliasBlockFile::BuildFromXML(DirManager &dm, const wxChar **attrs) else if (XMLValueChecker::IsGoodFileName(strValue, dm.GetProjectDataDir())) // Allow fallback of looking for the file name, located in the data directory. aliasFileName.Assign(dm.GetProjectDataDir(), strValue); - else - { + else if (XMLValueChecker::IsGoodPathString(strValue)) + // If the aliased file is missing, we failed XMLValueChecker::IsGoodPathName() + // and XMLValueChecker::IsGoodFileName, because both do existence tests, + // but we want to keep the reference to the missing file because it's a good path string. aliasFileName.Assign(strValue); - bMissingAliasFile = true; - } } else if (XMLValueChecker::IsGoodInt(strValue) && strValue.ToLong(&nValue)) { // integer parameters if( !wxStricmp(attr, wxT("aliasstart")) ) - aliasStart = nValue; + { + if (nValue >= 0) + aliasStart = nValue; + } else if( !wxStricmp(attr, wxT("aliaslen")) ) - aliasLen = nValue; + { + if (nValue >= 0) + aliasLen = nValue; + } else if( !wxStricmp(attr, wxT("aliaschannel")) ) - aliasChannel = nValue; + { + if (XMLValueChecker::IsValidChannel(aliasChannel)) + aliasChannel = nValue; + } else if( !wxStricmp(attr, wxT("min")) ) min = nValue; else if( !wxStricmp(attr, wxT("max")) ) max = nValue; else if( !wxStricmp(attr, wxT("rms")) ) - rms = nValue; + { + if (nValue >= 0) + rms = nValue; + } } //the min/max can be (are?) doubles as well, so handle this case: else if( !wxStrcmp(attr, wxT("min")) && @@ -252,33 +261,10 @@ BlockFile *PCMAliasBlockFile::BuildFromXML(DirManager &dm, const wxChar **attrs) } - if (!XMLValueChecker::IsGoodFileName( - summaryFileName.GetFullName(), - summaryFileName.GetPath(wxPATH_GET_VOLUME)) || - // Check the aliasFileName validity only if we do not already know file is missing. - (!bMissingAliasFile && - !XMLValueChecker::IsGoodFileName( - aliasFileName.GetFullName(), - aliasFileName.GetPath(wxPATH_GET_VOLUME))) || - (aliasStart < 0) || (aliasLen < 0) || - !XMLValueChecker::IsValidChannel(aliasChannel) || - (rms < 0.0)) - return NULL; - - // Even if we know aliasFileName is missing (bMissingAliasFile), + // Even if, for example, we know aliasFileName is blank, // create a PCMAliasBlockFile, so it gets put on DirManager's // mBlockFileHash and gets discovered in DirManager::ProjectFSCK(), // where user will get to decide what to do about it. - // Vaughan, 2010-07-28 (bug 113): - // Previously, the "aliasfile" clause above returned NULL. - // This caused Sequence::HandleXMLEndTag to create a SilentBlockFile, - // but that never got put on DirManager's mBlockFileHash, so - // DirManager::ProjectFSCK() never found the missing files. - // That caused FindDependencies to miss them, too, so a re-opened - // project with missing alias file(s) would show as self-contained, - // among other errors. - //vvv *Not sure whether that "replace missing blockfiles with SilentBlockFiles" - // loop in Sequence::HandleXMLEndTag can now be removed... return new PCMAliasBlockFile(summaryFileName, aliasFileName, aliasStart, aliasLen, aliasChannel, min, max, rms); diff --git a/src/xml/XMLTagHandler.cpp b/src/xml/XMLTagHandler.cpp index 1a60efb48..7df2c59f4 100644 --- a/src/xml/XMLTagHandler.cpp +++ b/src/xml/XMLTagHandler.cpp @@ -65,6 +65,18 @@ bool XMLValueChecker::IsGoodFileName(const wxString strFileName, const wxString return (fileName.IsOk() && fileName.FileExists()); } +bool XMLValueChecker::IsGoodFileString(wxString str) +{ + return (IsGoodString(str) && + !str.IsEmpty() && + + // FILENAME_MAX is 260 in MSVC, but inconsistent across platforms, + // sometimes huge, but we use 260 for all platforms. + (str.Length() <= 260) && + + (str.Find(wxFileName::GetPathSeparator()) == -1)); // No path separator characters. +} + bool XMLValueChecker::IsGoodSubdirName(const wxString strSubdirName, const wxString strDirName /* = "" */) { // Test strSubdirName. @@ -91,14 +103,18 @@ bool XMLValueChecker::IsGoodPathName(const wxString strPathName) return XMLValueChecker::IsGoodFileName(fileName.GetFullName(), fileName.GetPath(wxPATH_GET_VOLUME)); } -bool XMLValueChecker::IsGoodFileString(wxString str) +bool XMLValueChecker::IsGoodPathString(wxString str) { return (IsGoodString(str) && - !str.IsEmpty() && - (str.Length() <= 260) && // FILENAME_MAX is 260 in MSVC, but inconsistent across platforms, sometimes huge. - (str.Find(wxFileName::GetPathSeparator()) == -1)); // No path separator characters. + !str.IsEmpty() + + #ifdef _WIN32 + && (str.Length() <= MAX_PATH) + #endif + ); } + bool XMLValueChecker::IsGoodInt(const wxString strInt) { if (!IsGoodString(strInt)) @@ -189,15 +205,3 @@ XMLTagHandler *XMLTagHandler::ReadXMLChild(const char *tag) { return HandleXMLChild(UTF8CTOWX(tag).c_str()); } - -// Indentation settings for Vim and Emacs and unique identifier for Arch, a -// version control system. Please do not modify past this point. -// -// Local Variables: -// c-basic-offset: 3 -// indent-tabs-mode: nil -// End: -// -// vim: et sts=3 sw=3 -// arch-tag: 6aabae58-19bd-4b3a-aa6c-08432a7e106e - diff --git a/src/xml/XMLTagHandler.h b/src/xml/XMLTagHandler.h index 08ac888f1..9db19e31e 100644 --- a/src/xml/XMLTagHandler.h +++ b/src/xml/XMLTagHandler.h @@ -32,8 +32,10 @@ public: static bool IsGoodString(const wxString str); static bool IsGoodFileName(const wxString strFileName, const wxString strDirName = wxEmptyString); + static bool IsGoodFileString(wxString str); static bool IsGoodSubdirName(const wxString strSubdirName, const wxString strDirName = wxEmptyString); static bool IsGoodPathName(const wxString strPathName); + static bool IsGoodPathString(wxString str); // Note that because wxString::ToLong does additional testing, IsGoodInt doesn't duplicate // that testing, so use wxString::ToLong after IsGoodInt, not just atoi. @@ -41,8 +43,6 @@ public: static bool IsValidChannel(const int nValue); static bool IsValidSampleFormat(const int nValue); // true if nValue is one sampleFormat enum values - - static bool IsGoodFileString(wxString str); };