(bug 451, P2) Add more error checking and increase specificity of Sequence-reading code.

Add wxLogWarning messages to AudacityProject::OpenFile()

Fixed correction for "A linked track's partner should never itself be linked" to remove the link from the partner, not the original (left).

Fix possible NULL pointer dereference in previous commit.

src/DirManager.cpp

@@ -1044,11 +1044,14 @@ bool DirManager::HandleXMLTag(const wxChar *tag, const wxChar **attrs)
    }
    else
       return false;
-      
-   if ((pBlockFile == NULL) || 
-         // Check the length here so we don't have to do it in each BuildFromXML method.
-         ((mMaxSamples > -1) && // is initialized
-            (pBlockFile->GetLength() > mMaxSamples)))
+
+   if (!pBlockFile)
+      // BuildFromXML failed, or we didn't find a valid blockfile tag.
+      return false;
+
+   // Check the length here so we don't have to do it in each BuildFromXML method.
+   if ((mMaxSamples > -1) && // is initialized
+         (pBlockFile->GetLength() > mMaxSamples))
    {
       // See http://bugzilla.audacityteam.org/show_bug.cgi?id=451#c13. 
       // Lock pBlockFile so that the ~BlockFile() will not delete the file on disk. 
@@ -1350,17 +1353,6 @@ void DirManager::Deref()
 // recent savefile.
 int DirManager::ProjectFSCK(const bool bForceError, const bool bAutoRecoverMode)
 {
-   wxGetApp().SetMissingAliasedFileWarningShouldShow(false);
-   wxArrayString filePathArray; // *all* files in the project directory/subdirectories
-   wxString dirPath = (projFull != wxT("") ? projFull : mytemp);
-   RecursivelyEnumerateWithProgress(
-      dirPath, 
-      filePathArray,          // output: all files in project directory tree
-      wxEmptyString, 
-      true, false, 
-      mBlockFileHash.size(),  // rough guess of how many BlockFiles will be found/processed, for progress
-      _("Inspecting project file data"));
-
    // In earlier versions of this method, enumerations of errors were 
    // all done in sequence, then the user was prompted for each type of error. 
    // The enumerations are now interleaved with prompting, because, for example, 
@@ -1373,15 +1365,40 @@ int DirManager::ProjectFSCK(const bool bForceError, const bool bAutoRecoverMode)
    int action; // choice of action for each type of error
    int nResult = 0;
    
+   if (bForceError && !bAutoRecoverMode)
+   {
+      wxString msg = _("Project check read faulty Sequence tags.");
+      const wxChar *buttons[] = 
+         {_("Close project immediately with no changes"),
+            _("Continue with repairs already noted in log, and check for more errors"),
+            NULL};
+      wxLog::FlushActive(); // MultiDialog has "Show Log..." button, so make sure log is current.
+      action = ShowMultiDialog(msg, _("Warning - Problems Reading Sequence Tags"), buttons);
+      if (action == 0) 
+         nResult = FSCKstatus_CLOSE_REQ;
+      else 
+         nResult = FSCKstatus_CHANGED | FSCKstatus_SAVE_AUP; 
+   }
+
+   wxArrayString filePathArray; // *all* files in the project directory/subdirectories
+   wxString dirPath = (projFull != wxT("") ? projFull : mytemp);
+   RecursivelyEnumerateWithProgress(
+      dirPath, 
+      filePathArray,          // output: all files in project directory tree
+      wxEmptyString, 
+      true, false, 
+      mBlockFileHash.size(),  // rough guess of how many BlockFiles will be found/processed, for progress
+      _("Inspecting project file data"));
+
    //
    // MISSING ALIASED AUDIO FILES
    //
+   wxGetApp().SetMissingAliasedFileWarningShouldShow(false);
    BlockHash missingAliasedFileAUFHash;   // (.auf) AliasBlockFiles whose aliased files are missing
    BlockHash missingAliasedFilePathHash;  // full paths of missing aliased files
    this->FindMissingAliasedFiles(missingAliasedFileAUFHash, missingAliasedFilePathHash);
 
-   // No need to check (nResult != FSCKstatus_CLOSE_REQ) as this is first check.
-   if (!missingAliasedFileAUFHash.empty()) 
+   if ((nResult != FSCKstatus_CLOSE_REQ) && !missingAliasedFileAUFHash.empty()) 
    {
       // In auto-recover mode, we always create silent blocks, and do not ask user.
       // This makes sure the project is complete next time we open it.
@@ -1480,7 +1497,7 @@ _("Project check detected %d missing alias (.auf) \
             if(action==0){
                //regenerate from data
                b->Recover();
-               nResult = FSCKstatus_CHANGED; 
+               nResult |= FSCKstatus_CHANGED; 
             }else if (action==1){
                // Silence error logging for this block in this session.
                b->SilenceLog(); 
@@ -1595,7 +1612,6 @@ _("Project check found %d orphan blockfile(s). These files are \
       }
    }
 
-   if (nResult != FSCKstatus_CLOSE_REQ)
    if ((nResult != FSCKstatus_CLOSE_REQ) && !ODManager::HasLoadedODFlag())
    {
       // Remove any empty directories.

src/Project.cpp

@@ -2423,7 +2423,12 @@ void AudacityProject::OpenFile(wxString fileName, bool addtohistory)
       t = iter.First();
       while (t) {
          if (t->GetErrorOpening())
+         {
+            wxLogWarning(
+               wxT("Track %s had error reading clip values from project file."), 
+               t->GetName().c_str());
             err = true;
+         }
 
          // Sanity checks for linked tracks; unsetting the linked property
          // doesn't fix the problem, but it likely leaves us with orphaned
@@ -2436,8 +2441,11 @@ void AudacityProject::OpenFile(wxString fileName, bool addtohistory)
                // A linked track's partner should never itself be linked
                if (l->GetLinked())
                {
+                  wxLogWarning(
+                     wxT("Left track %s had linked right track %s with extra right track link.\n   Removing extra link from right track."), 
+                     t->GetName().c_str(), l->GetName().c_str());
                   err = true;
-                  t->SetLinked(false);
+                  l->SetLinked(false);
                }
                
                // Channels should be left and right
@@ -2446,12 +2454,18 @@ void AudacityProject::OpenFile(wxString fileName, bool addtohistory)
                         (t->GetChannel() == Track::RightChannel &&
                            l->GetChannel() == Track::LeftChannel) ) )
                {
+                  wxLogWarning(
+                     wxT("Track %s and %s had left/right track links out of order. Setting tracks to not be linked."), 
+                     t->GetName().c_str(), l->GetName().c_str());
                   err = true;
                   t->SetLinked(false);
                }
             }
             else
             {
+               wxLogWarning(
+                  wxT("Track %s had link to NULL track. Setting it to not be linked."), 
+                  t->GetName().c_str());
                err = true;
                t->SetLinked(false);
             }

src/Sequence.cpp

@@ -733,6 +733,9 @@ bool Sequence::HandleXMLTag(const wxChar *tag, const wxChar **attrs)
          {
             delete (wb);
             mErrorOpening = true;
+            wxLogWarning(
+               wxT("   Sequence has bad %s attribute value, %s, that should be a positive integer."), 
+               attr, strValue.c_str());
             return false;
          }
          
@@ -843,12 +846,15 @@ void Sequence::HandleXMLEndTag(const wxChar *tag)
          {
          	// This could be why the blockfile failed, so limit 
          	// the silent replacement to mMaxSamples.
+            wxLogWarning(
+               wxT("   Sequence has missing block file with length %s > mMaxSamples %s. Setting length to mMaxSamples."), 
+               Internat::ToString(((wxLongLong)len).ToDouble(), 0), 
+               Internat::ToString(((wxLongLong)mMaxSamples).ToDouble(), 0));
             len = mMaxSamples;
-            wxLogWarning(_("   Sequence has missing block file with length > mMaxSamples."));
          }
          mBlock->Item(b)->f = new SilentBlockFile(len);
          wxLogWarning(
-            _("Gap detected in project file. Missing block file replaced with silence."));
+            wxT("Gap detected in project file. Replacing missing block file with silence."));
          mErrorOpening = true;
       }
    }
@@ -857,21 +863,27 @@ void Sequence::HandleXMLEndTag(const wxChar *tag)
    sampleCount numSamples = 0;
    for (b = 0; b < mBlock->Count(); b++) {
       if (mBlock->Item(b)->start != numSamples) {
-         mBlock->Item(b)->start = numSamples;
          wxString sFileAndExtension = mBlock->Item(b)->f->GetFileName().GetFullName();
          if (sFileAndExtension.IsEmpty())
-            sFileAndExtension = _("(replaced with silence)");
+            sFileAndExtension = wxT("(replaced with silence)");
          else 
             sFileAndExtension = wxT("\"") + sFileAndExtension + wxT("\"");
-         wxLogWarning(_("Gap detected in project file.\n   Start for block file %s is more than one sample past end of previous block.\n   Start has been moved back so blocks are contiguous."), 
-                     sFileAndExtension);
+         wxLogWarning(
+            wxT("Gap detected in project file.\n   Start (%s) for block file %s is more than one sample past end of previous block (%s).\n   Moving start back so blocks are contiguous."), 
+            Internat::ToString(((wxLongLong)(mBlock->Item(b)->start)).ToDouble(), 0), 
+            sFileAndExtension, 
+            Internat::ToString(((wxLongLong)(numSamples)).ToDouble(), 0));
+         mBlock->Item(b)->start = numSamples;
          mErrorOpening = true;         
       }
       numSamples += mBlock->Item(b)->f->GetLength();
    }
    if (mNumSamples != numSamples) {
+      wxLogWarning(
+         wxT("Gap detected in project file. Correcting sequence sample count from %s to %s."), 
+         Internat::ToString(((wxLongLong)mNumSamples).ToDouble(), 0), 
+         Internat::ToString(((wxLongLong)numSamples).ToDouble(), 0));
       mNumSamples = numSamples;
-      wxLogWarning(_("Gap detected in project file. Sequence sample count corrected."));
       mErrorOpening = true;
    }
 }