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

@@ -548,8 +548,8 @@ wxFileName DirManager::MakeBlockFilePath(wxString value){
    return dir;
 }
 
-bool DirManager::AssignFile(wxFileName &fileName,
+bool DirManager::AssignFile(wxFileName &fileName, 
-			    wxString value,
+                            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++;

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))
+         if (XMLValueChecker::IsGoodFileString(strValue))
-            return NULL;
+         {
-
+            #ifdef _WIN32
-         #ifdef _WIN32
+               if (strValue.Length() + 1 + dm.GetProjectDataDir().Length() <= MAX_PATH)
-            if (strValue.Length() + 1 + dm.GetProjectDataDir().Length() > MAX_PATH)
+            #endif
-               return NULL;
+                  dm.AssignFile(summaryFileName, strValue, false);
-         #endif
+         }
-
-         dm.AssignFile(summaryFileName,value,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(
+   // Even if, for example, we know aliasFileName is blank, 
-         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), 
    // 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);

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.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. 
+            #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
-

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);
 };