From f1fee888c2e48077e4dd8f7ce5b7fa1b61aef1f4 Mon Sep 17 00:00:00 2001 From: "v.audacity" Date: Thu, 24 Nov 2011 23:12:52 +0000 Subject: [PATCH] Fix null pointer dereferences caught by Vigilant Sentry. Ask some questions about some code. --- src/LabelDialog.cpp | 3 +++ src/Menus.cpp | 4 +++- src/TrackArtist.cpp | 22 ++++++++++++---------- src/TrackPanel.cpp | 28 ++++++++++++++-------------- 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/LabelDialog.cpp b/src/LabelDialog.cpp index 15b271eee..44479a147 100644 --- a/src/LabelDialog.cpp +++ b/src/LabelDialog.cpp @@ -330,6 +330,9 @@ bool LabelDialog::TransferDataFromWindow() break; } } + wxASSERT(t); + if (!t) + return false; // Add the label to it if (!rd->title.IsEmpty()) { diff --git a/src/Menus.cpp b/src/Menus.cpp index 467e0f32d..c1954ac82 100644 --- a/src/Menus.cpp +++ b/src/Menus.cpp @@ -2967,7 +2967,9 @@ void AudacityProject::OnExportMIDI(){ return; } - assert(nt); + wxASSERT(nt); + if (!nt) + return; while(true){ diff --git a/src/TrackArtist.cpp b/src/TrackArtist.cpp index f70be11aa..88a4aaac8 100644 --- a/src/TrackArtist.cpp +++ b/src/TrackArtist.cpp @@ -969,7 +969,8 @@ void TrackArtist::DrawMinMaxRMS(wxDC &dc, const wxRect &r, const double env[], double v; v = min[x] * env[x]; - if (mShowClipping && v <= -MAX_AUDIO) { + if (clipped && mShowClipping && (v <= -MAX_AUDIO)) + { if (clipcnt == 0 || clipped[clipcnt - 1] != xx) { clipped[clipcnt++] = xx; } @@ -978,7 +979,8 @@ void TrackArtist::DrawMinMaxRMS(wxDC &dc, const wxRect &r, const double env[], r.height, dB, true, mdBrange, true); v = max[x] * env[x]; - if (mShowClipping && v >= MAX_AUDIO) { + if (clipped && mShowClipping && (v >= MAX_AUDIO)) + { if (clipcnt == 0 || clipped[clipcnt - 1] != xx) { clipped[clipcnt++] = xx; } @@ -1128,9 +1130,8 @@ void TrackArtist::DrawIndividualSamples(wxDC &dc, const wxRect &r, // t0 + clip->GetOffset() is 'h' (the absolute time of the left edge) for 'r'. tt = buffer[s] * clip->GetEnvelope()->GetValueAtX(xx + r.x, r, t0 + clip->GetOffset(), pps); - if (mShowClipping && (tt <= -MAX_AUDIO || tt >= MAX_AUDIO)) { + if (clipped && mShowClipping && ((tt <= -MAX_AUDIO) || (tt >= MAX_AUDIO))) clipped[clipcnt++] = xx; - } ypos[s] = GetWaveYPos(tt, zoomMin, zoomMax, r.height, dB, true, mdBrange, false); if (ypos[s] < -1) { @@ -2422,12 +2423,13 @@ void TrackArtist::DrawNoteTrack(NoteTrack *track, Alg_seq_ptr seq = track->mSeq; if (!seq) { assert(track->mSerializationBuffer); - Alg_track_ptr alg_track = seq->unserialize(track->mSerializationBuffer, - track->mSerializationLength); - assert(alg_track->get_type() == 's'); - track->mSeq = seq = (Alg_seq_ptr) alg_track; - free(track->mSerializationBuffer); - track->mSerializationBuffer = NULL; + // FIX-ME: This is in a clause where we *know* seq is NULL, so why are you dereferencing it?!!! I'm commenting out the rest of this clause. + //Alg_track_ptr alg_track = seq->unserialize(track->mSerializationBuffer, + // track->mSerializationLength); + //assert(alg_track->get_type() == 's'); + //track->mSeq = seq = (Alg_seq_ptr) alg_track; + //free(track->mSerializationBuffer); + //track->mSerializationBuffer = NULL; } assert(seq); int visibleChannels = track->mVisibleChannels; diff --git a/src/TrackPanel.cpp b/src/TrackPanel.cpp index b2aeb4847..97b66935d 100644 --- a/src/TrackPanel.cpp +++ b/src/TrackPanel.cpp @@ -1970,7 +1970,8 @@ void TrackPanel::SelectionHandleClick(wxMouseEvent & event, } //Determine if user clicked on a label track. - if (pTrack->GetKind() == Track::Label) { + if (pTrack && (pTrack->GetKind() == Track::Label)) + { LabelTrack *lt = (LabelTrack *) pTrack; if (lt->HandleMouse(event, r,//mCapturedRect, mViewInfo->h, mViewInfo->zoom, @@ -2164,10 +2165,8 @@ void TrackPanel::ExtendSelection(int mouseXCoordinate, int trackLeftEdge, mViewInfo->sel1 = sel1; //On-Demand: check to see if there is an OD thing associated with this track. If so we want to update the focal point for the task. - if (pTrack->GetKind() == Track::Wave) { - if(ODManager::IsInstanceCreated()) - ODManager::Instance()->DemandTrackUpdate((WaveTrack*)pTrack,sel0); //sel0 is sometimes less than mSelStart - } + if (pTrack && (pTrack->GetKind() == Track::Wave) && ODManager::IsInstanceCreated()) + ODManager::Instance()->DemandTrackUpdate((WaveTrack*)pTrack,sel0); //sel0 is sometimes less than mSelStart // Full refresh since the label area may need to indicate // newly selected tracks. @@ -4524,7 +4523,8 @@ void TrackPanel::HandleResizeDrag(wxMouseEvent & event) // minimized heights. if (mCapturedTrack->GetLinked()) { mInitialUpperTrackHeight = mCapturedTrack->GetHeight(); - mInitialTrackHeight = link->GetHeight(); + if (link) // FIX-ME: This wasn't checked previously. Is mInitialTrackHeight safe if this doesn't fire? + mInitialTrackHeight = link->GetHeight(); } else if (link) { mInitialUpperTrackHeight = link->GetHeight(); @@ -5882,12 +5882,8 @@ void TrackPanel::UpdateVRulerSize() /// TrackPanel::OnNextTrack. void TrackPanel::OnPrevTrack( bool shift ) { - Track *t; - Track *p; TrackListIterator iter( mTracks ); - bool tSelected,pSelected; - - t = GetFocusedTrack(); // Get currently focused track + Track* t = GetFocusedTrack(); if( t == NULL ) // if there isn't one, focus on last { t = iter.Last(); @@ -5896,18 +5892,21 @@ void TrackPanel::OnPrevTrack( bool shift ) return; } + Track* p = NULL; + bool tSelected = false; + bool pSelected = false; if( shift ) { p = mTracks->GetPrev( t, true ); // Get previous track if( p == NULL ) // On first track { - wxBell(); + wxBell(); // ANSWER-ME: Why? if( mCircularTrackNavigation ) { TrackListIterator iter( mTracks ); for( Track *d = iter.First(); d; d = iter.Next( true ) ) { - p = d; + p = d; // ANSWER-ME: Is there supposed to be a stopping condition here? If seeking last, why not just use iter.Last()? And what does "d" stand for?! } } else @@ -5917,7 +5916,8 @@ void TrackPanel::OnPrevTrack( bool shift ) } } tSelected = t->GetSelected(); - pSelected = p->GetSelected(); + if (p) + pSelected = p->GetSelected(); if( tSelected && pSelected ) { mTracks->Select( t, false );