audacia
/
audacia
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fixes per Vigilant Sentry (http://www.vigilantsw.com/) 

* Fix memory leaks.

* Add comments about initializations and checking for successful results.

* Add checks for NULL deref.

* Consistency in "TODO" vs "TO-DO" comments!
This commit is contained in:
v.audacity 2012-02-08 05:09:14 +00:00
parent 5b3f3f71ea
commit 5f5b9778de
23 changed files with 83 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/AudioIO.cpp
View File

@ -978,7 +978,10 @@ bool AudioIO::StartPortAudioStream(double sampleRate,
playbackDeviceInfo = Pa_GetDeviceInfo( playbackParameters->device );
if( playbackDeviceInfo == NULL )
{
delete playbackParameters;
return false;
}
// regardless of source formats, we always mix to float
playbackParameters->sampleFormat = paFloat32;
@ -1005,7 +1008,11 @@ bool AudioIO::StartPortAudioStream(double sampleRate,
captureDeviceInfo = Pa_GetDeviceInfo( captureParameters->device );
if( captureDeviceInfo == NULL )
{
delete captureParameters;
delete playbackParameters;
return false;
}
captureParameters->sampleFormat =
AudacityToPortAudioSampleFormat(mCaptureFormat);
@ -1087,6 +1094,7 @@ void AudioIO::StartMonitoring(double sampleRate)
success = StartPortAudioStream(sampleRate, (unsigned int)playbackChannels,
(unsigned int)captureChannels,
captureFormat);
// TODO: Check return value of success.
// Now start the PortAudio stream!
mLastPaError = Pa_StartStream( mPortStreamV19 );

3
src/AutoRecovery.cpp
View File

@ -220,7 +220,8 @@ static bool RecoverAllProjects(AudacityProject** pproj)
bool ShowAutoRecoveryDialogIfNeeded(AudacityProject** pproj,
bool *didRecoverAnything)
{
*didRecoverAnything = false;
if (didRecoverAnything)
*didRecoverAnything = false;
if (HaveFilesToRecover())
{
AutoRecoveryDialog dlg(*pproj);

3
src/DeviceManager.cpp
View File

@ -270,12 +270,11 @@ void DeviceManager::Rescan()
}
int nDevices = Pa_GetDeviceCount();
int i;
//The heirarchy for devices is Host/device/source.
//Some newer systems aggregate this.
//So we need to call port mixer for every device to get the sources
for (i = 0; i < nDevices; i++) {
for (int i = 0; i < nDevices; i++) {
const PaDeviceInfo *info = Pa_GetDeviceInfo(i);
if (info->maxOutputChannels > 0) {
AddSources(i, info->defaultSampleRate, &mOutputDeviceSourceMaps, 0);

8
src/FreqWindow.cpp
View File

@ -524,7 +524,7 @@ void FreqWindow::DrawPlot()
xMax = mRate / 2;
xRatio = xMax / xMin;
xPos = xMin;
xLast = xPos / 2.0;
xLast = xPos / 2.0; // ANSWER-ME: Vigilant Sentry notes this var is unused after this assignment. Delete it and the var decl?
if (mLogAxis)
{
xStep = pow(2.0f, (log(xRatio) / log(2.0f)) / width);
@ -540,7 +540,7 @@ void FreqWindow::DrawPlot()
xMin = 0;
xMax = mProcessedSize / mRate;
xPos = xMin;
xLast = xPos / 2.0;
xLast = xPos / 2.0; // ANSWER-ME: Vigilant Sentry notes this var is unused after this assignment. Delete it and the var decl?
xStep = (xMax - xMin) / width;
hRuler->ruler.SetLog(false);
hRuler->ruler.SetUnits(_("s"));
@ -787,7 +787,7 @@ void FreqWindow::PlotPaint(wxPaintEvent & evt)
xMax = mRate / 2;
xRatio = xMax / xMin;
xPos = xMin;
xLast = xPos / 2.0;
xLast = xPos / 2.0; // ANSWER-ME: Vigilant Sentry notes this var is unused after this assignment. Delete it and the var decl?
if (mLogAxis)
xStep = pow(2.0f, (log(xRatio) / log(2.0f)) / width);
else
@ -796,7 +796,7 @@ void FreqWindow::PlotPaint(wxPaintEvent & evt)
xMin = 0;
xMax = mProcessedSize / mRate;
xPos = xMin;
xLast = xPos / 2.0;
xLast = xPos / 2.0; // ANSWER-ME: Vigilant Sentry notes this var is unused after this assignment. Delete it and the var decl?
xStep = (xMax - xMin) / width;
}

2
src/LabelTrack.cpp
View File

@ -1220,6 +1220,8 @@ void LabelStruct::AdjustEdge( int iEdge, double fNewTime)
// We're moving the label. Adjust both left and right edge.
void LabelStruct::MoveLabel( int iEdge, double fNewTime)
{
// ANSWER-ME: Vigilant Sentry notes this "width" shadows the member var of same name.
// Is that what we actually want?
double width = getDuration();
if( iEdge < 0 )

4
src/Menus.cpp
View File

@ -5573,6 +5573,8 @@ void AudacityProject::OnExportCleanSpeechPresets()
int lenPreset = sizeof(preset);
int count = presetsFile.Write(preset, lenPreset);
// ANSWER-ME: Vigilant Sentry notes "count" is unused after this assignment.
// Should be checked against expectedCount -- but CleanSpeech is going away!!! ;-)
count = presetsFile.Write(pNoiseGate, expectedCount);
presetsFile.Close();
@ -5628,6 +5630,8 @@ void AudacityProject::OnImportCleanSpeechPresets()
}
int expectedCount = wxGetApp().GetCleanSpeechNoiseGateExpectedCount();
float* pNoiseGate = wxGetApp().GetCleanSpeechNoiseGate();
// ANSWER-ME: Vigilant Sentry notes "count" is unused after this assignment.
// Should be checked against expectedCount -- but CleanSpeech is going away!!! ;-)
count = presetsFile.Read(pNoiseGate, expectedCount);
gPrefs->Write(wxT("/CsPresets/ClickThresholdLevel"), preset[2]);

13
src/Sequence.cpp
View File

@ -181,10 +181,10 @@ bool Sequence::ConvertToSampleFormat(sampleFormat format, bool* pbChanged)
if (pOldBlockFile->IsAlias())
{
// No conversion of aliased data.
//vvvvv Should the user be alerted, as we're not actually converting the aliased file?
//v Should the user be alerted, as we're not actually converting the aliased file?
// James (2011-12-01, offlist) believes this is okay because we are assuring
// the user we'll do the format conversion if we turn this into a non-aliased block.
// TO-DO: Confirm that.
// TODO: Confirm that.
pNewBlockArray->Add(pOldSeqBlock);
}
else
@ -511,6 +511,7 @@ bool Sequence::Paste(sampleCount s, const Sequence *src)
return ConsistencyCheck(wxT("Paste branch one"));
}
// FIX-ME: "b" is unsigned, so it's pointless to check that it's >= 0.
if (b >= 0 && b < numBlocks
&& ((mBlock->Item(b)->f->GetLength() + addedLen) < mMaxSamples)) {
// Special case: we can fit all of the new samples inside of
@ -626,8 +627,10 @@ bool Sequence::Paste(sampleCount s, const Sequence *src)
insertBlock->f = mDirManager->CopyBlockFile(srcBlock->Item(i)->f);
if (!insertBlock->f) {
// TODO error: Could not paste! (Out of disk space?)
wxASSERT(false);
wxASSERT(false); // TODO: Handle this better, alert the user of failure.
delete insertBlock;
newBlock->Clear();
delete newBlock;
return false;
}
@ -772,6 +775,8 @@ bool Sequence::AppendBlock(SeqBlock * b)
newBlock->f = mDirManager->CopyBlockFile(b->f);
if (!newBlock->f) {
/// \todo Error Could not paste! (Out of disk space?)
wxASSERT(false); // TODO: Handle this better, alert the user of failure.
delete newBlock;
return false;
}

2
src/WaveClip.cpp
View File

@ -997,7 +997,7 @@ void WaveClip::ConvertToSampleFormat(sampleFormat format)
bool bResult = mSequence->ConvertToSampleFormat(format, &bChanged);
if (bResult && bChanged)
MarkChanged();
wxASSERT(bResult); // TO-DO: Throw an actual error.
wxASSERT(bResult); // TODO: Throw an actual error.
}
void WaveClip::UpdateEnvelopeTrackLen()

2
src/blockfile/ODDecodeBlockFile.cpp
View File

@ -336,7 +336,7 @@ int ODDecodeBlockFile::WriteODDecodeBlockFile()
mLen,
mFormat,
NULL);//summaryData);
wxASSERT(bSuccess); // TO-DO: Handle failure here by alert to user and undo partial op.
wxASSERT(bSuccess); // TODO: Handle failure here by alert to user and undo partial op.
mFileNameMutex.Unlock();

2
src/blockfile/SimpleBlockFile.cpp
View File

@ -111,7 +111,7 @@ SimpleBlockFile::SimpleBlockFile(wxFileName baseFileName,
if (!(allowDeferredWrite && useCache) && !bypassCache)
{
bool bSuccess = WriteSimpleBlockFile(sampleData, sampleLen, format, NULL);
wxASSERT(bSuccess); // TO-DO: Handle failure here by alert to user and undo partial op.
wxASSERT(bSuccess); // TODO: Handle failure here by alert to user and undo partial op.
}
if (useCache) {

2
src/effects/AutoDuck.cpp
View File

@ -436,6 +436,7 @@ bool EffectAutoDuck::ApplyDuckFade(int trackNumber, WaveTrack* t,
}
}
delete[] buf;
return cancel;
}
@ -954,6 +955,7 @@ void EffectAutoDuckPanel::OnMotion(wxMouseEvent &evt)
else
dist = abs(evt.GetX() -
mMoveStartControlPoints[mCurrentControlPoint].x);
// TODO: Get rid of unused 'dist' var within this scope?
float newValue;

14
src/effects/Equalization.cpp
View File

@ -2056,7 +2056,11 @@ void EqualizationDialog::LayoutEQSliders()
void EqualizationDialog::GraphicEQ(Envelope *env)
{
double value, dist, span, s;
// Vigilant Sentry noted this "value" var had not previously been initialized,
// so there must be some path to lines 2133 and 2179 that do not get a setting.
// ANSWER-ME: Is this a good default value?
double value = 0.0;
double dist, span, s;
env->Flatten(0.);
env->SetTrackLen(1.0);
@ -2981,7 +2985,7 @@ void EditCurvesDialog::OnRename(wxCommandEvent &event)
{
wxString name;
int numCurves = mEditCurves.GetCount();
int curve;
int curve = 0;
// Setup list of characters that aren't allowed
wxArrayString exclude;
@ -3052,6 +3056,11 @@ void EditCurvesDialog::OnRename(wxCommandEvent &event)
{
if(overwrite)
{ // Overwrite another curve with 'unnamed'
// ANSWER-ME: What is the expected value of "curve" here?
// It was not previously initialized at declaration.
// And if we expect it to have been through the above loop,
// it will be numCurves, which is a bad index, right?
// I've initialized it to zero, so we at least know what value it has.
mEditCurves[ curve ].Name = name;
mList->SetItem(curve, 0, name);
mEditCurves[ curve ].points = mEditCurves[ item ].points;
@ -3070,6 +3079,7 @@ void EditCurvesDialog::OnRename(wxCommandEvent &event)
{
if(overwrite)
{ // Overwrite another curve with this one, then delete this one
// ANSWER-ME: Same question as above, i.e., what's the expected value of "curve" here?
mEditCurves[ curve ].Name = name;
mEditCurves[ curve ].points = mEditCurves[ item ].points;
mEditCurves.RemoveAt( item );

2
src/effects/vamp/VampEffect.cpp
View File

@ -221,7 +221,7 @@ bool VampEffect::Process()
mTracks->Add(ltrack);
float **data = new float*[channels];
float **data = new float*[channels]; // ANSWER-ME: Vigilant Sentry marks this as memory leak, var "data" not deleted.
for (int c = 0; c < channels; ++c) data[c] = new float[block];
sampleCount originalLen = len;

3
src/export/Export.cpp
View File

@ -640,10 +640,9 @@ bool Exporter::GetFilename()
// This causes problems for the exporter, so we don't allow it.
// Overwritting non-missing aliased files is okay.
// Also, this can only happen for uncompressed audio.
size_t i;
bool overwritingMissingAlias;
overwritingMissingAlias = false;
for (i = 0; i < gAudacityProjects.GetCount(); i++) {
for (size_t i = 0; i < gAudacityProjects.GetCount(); i++) {
AliasedFileArray aliasedFiles;
FindDependencies(gAudacityProjects[i], &aliasedFiles);
size_t j;

6
src/import/Import.cpp
View File

@ -282,7 +282,11 @@ void Importer::WriteImportItems()
gPrefs->Write (name, val);
}
/* If we had more items than we have now, delete the excess */
for (i = this->mExtImportItems->Count(); i >= 0; i++)
// ANSWER-ME: Vigilant Sentry notes i is unsigned so (i >= 0) is always true.
// If that's what you want, why not make the condition just be "true"? Or should it be an int?
// But also, it looks like it's supposed to be a down-counting loop, so is i++ correct?
// Rather, shouldn't i be set to this->mExtImportItems->Count() each time?
for (i = this->mExtImportItems->Count(); i >= 0; i++)
{
name.Printf (wxT("/ExtImportItems/Item%d"), i);
if (gPrefs->Read(name, &val))

17
src/import/ImportFLAC.cpp
View File

@ -467,13 +467,16 @@ int FLACImportFileHandle::Import(TrackFactory *trackFactory,
useOD=true;
#endif
#ifdef LEGACY_FLAC
bool res = (mFile->process_until_end_of_file() != 0);
#else
bool res = true;
if(!useOD)
res = (mFile->process_until_end_of_stream() != 0);
#endif
// ANSWER-ME: Vigilant Sentry: Variable res unused after assignment (error code DA1)
// Plus, it's not even declared in the LEGACY_FLAC clause
//#ifdef LEGACY_FLAC
// bool res = (mFile->process_until_end_of_file() != 0);
//#else
// bool res = true;
// if(!useOD)
// res = (mFile->process_until_end_of_stream() != 0);
//#endif
//add the task to the ODManager
if(useOD)
{

1
src/import/ImportMIDI.cpp
View File

@ -53,6 +53,7 @@ bool ImportMIDI(wxString fName, NoteTrack * dest)
if(new_seq->get_read_error() == alg_error_open){
wxMessageBox( _("Could not open file ") + fName + wxT("."));
mf.Close();
delete new_seq;
return false;
}

2
src/ondemand/ODWaveTrackTaskQueue.cpp
View File

@ -225,6 +225,7 @@ WaveTrack* ODWaveTrackTaskQueue::GetWaveTrack(size_t x)
{
WaveTrack* ret = NULL;
mTracksMutex.Lock();
// FIX-ME: x is unsigned so there's no point in checking that it's >= 0.
if(x>=0&&x<mTracks.size())
ret = mTracks[x];
mTracksMutex.Unlock();
@ -256,6 +257,7 @@ ODTask* ODWaveTrackTaskQueue::GetTask(size_t x)
{
ODTask* ret = NULL;
mTasksMutex.Lock();
// FIX-ME: x is unsigned so there's no point in checking that it's >= 0.
if(x>=0&&x<mTasks.size())
ret = mTasks[x];
mTasksMutex.Unlock();

6
src/prefs/KeyConfigPrefs.cpp
View File

@ -391,6 +391,8 @@ void KeyConfigPrefs::SetKeyForSelected( const wxString & key )
void KeyConfigPrefs::OnSet(wxCommandEvent & e)
{
// ANSWER-ME: mCommandSelected is unsigned, so there's no point checking whether it's < 0.
// Should it be a signed int instead or remove that check?
if (mCommandSelected < 0 || mCommandSelected >= mNames.GetCount())
return;
@ -414,6 +416,8 @@ void KeyConfigPrefs::OnSet(wxCommandEvent & e)
void KeyConfigPrefs::OnClear(wxCommandEvent& event)
{
mKey->Clear();
// ANSWER-ME: mCommandSelected is unsigned, so there's no point checking whether it's < 0.
// Should it be a signed int instead or remove that check?
if (mCommandSelected < 0 || mCommandSelected >= mNames.GetCount()) {
return;
}
@ -500,6 +504,8 @@ void KeyConfigPrefs::OnCategory(wxCommandEvent & e)
void KeyConfigPrefs::OnItemSelected(wxListEvent & e)
{
mCommandSelected = e.GetIndex();
// ANSWER-ME: mCommandSelected is unsigned, so there's no point checking whether it's < 0.
// Should it be a signed int instead or remove that check?
if (mCommandSelected < 0 || mCommandSelected >= mNames.GetCount()) {
mKey->SetLabel(wxT(""));
return;

2
src/prefs/PrefsDialog.cpp
View File

@ -249,7 +249,7 @@ void PrefsDialog::OnOK(wxCommandEvent & event)
// set AudioIONotBusyFlag/AudioIOBusyFlag according to monitoring, as well.
// Instead allow it because unlike recording, for example, monitoring
// is not clearly something that should prohibit opening prefs.
// TO-DO: We *could* be smarter in this method and call HandleDeviceChange()
// TODO: We *could* be smarter in this method and call HandleDeviceChange()
// only when the device choices actually changed. True of lots of prefs!
// As is, we always stop monitoring before handling the device change.
if (gAudioIO->IsMonitoring())

7
src/toolbars/ControlToolBar.cpp
View File

@ -742,7 +742,8 @@ void ControlToolBar::StopPlaying(bool stopStream /* = true*/)
void ControlToolBar::OnBatch(wxCommandEvent &evt)
{
AudacityProject *proj = GetActiveProject();
proj->OnApplyChain();
if (proj)
proj->OnApplyChain();
mPlay->Enable();
mStop->Enable();
@ -762,8 +763,7 @@ void ControlToolBar::OnRecord(wxCommandEvent &evt)
AudacityProject *p = GetActiveProject();
if (p && p->GetCleanSpeechMode()) {
size_t numProjects = gAudacityProjects.Count();
bool tracks = (p && !p->GetTracks()->IsEmpty());
if (tracks || (numProjects > 1)) {
if (!p->GetTracks()->IsEmpty() || (numProjects > 1)) {
wxMessageBox(_("Recording in CleanSpeech mode is not possible when a track, or more than one project, is already open."),
_("Recording not permitted"),
wxOK | wxICON_INFORMATION,
@ -817,7 +817,6 @@ void ControlToolBar::OnRecord(wxCommandEvent &evt)
int recordingChannels = 0;
bool shifted = mRecord->WasShiftDown();
if (shifted) {
TrackListIterator it(t);
WaveTrack *wt;
bool sel = false;
double allt0 = t0;

2
src/toolbars/DeviceToolBar.cpp
View File

@ -717,7 +717,7 @@ void DeviceToolBar::OnChoice(wxCommandEvent &event)
// set AudioIONotBusyFlag/AudioIOBusyFlag according to monitoring, as well.
// Instead allow it because unlike recording, for example, monitoring
// is not clearly something that should prohibit changing device.
// TO-DO: We *could* be smarter in this method and call HandleDeviceChange()
// TODO: We *could* be smarter in this method and call HandleDeviceChange()
// only when the device choices actually changed. True of lots of prefs!
// As is, we always stop monitoring before handling the device change.
if (gAudioIO->IsMonitoring())

6
src/widgets/ErrorDialog.cpp
View File

@ -301,6 +301,8 @@ void ShowModelessErrorDialog(wxWindow *parent,
ErrorDialog *dlog = new ErrorDialog(parent, dlogTitle, message, helpURL, Close, false);
dlog->CentreOnParent();
dlog->Show();
// ANSWER-ME: Vigilant Sentry flags this method as not deleting dlog, so a mem leak.
// ANSWER-ME: This is unused. Delete it or are there plans for it?
}
void ShowAliasMissingDialog(AudacityProject *parent,
@ -321,8 +323,10 @@ void ShowAliasMissingDialog(AudacityProject *parent,
point.y = 100;
dlog->SetPosition(point);
dlog->CentreOnParent(wxHORIZONTAL);
dlog->Show();
// ANSWER-ME: Vigilant Sentry flags this method as not deleting dlog, so a mem leak.
// ANSWER-ME: Why is this modeless? Shouldn't it require user action before proceeding?
}
/// Mostly we use this so that we have the code for resizability