Autres/audacity
1
0
Fork 0
mirror of https://github.com/cookiengineer/audacity synced 2025-06-17 16:40:07 +02:00
Code Issues Releases Wiki Activity

Sqlite errors (#601)

Browse Source 
* Null checks on return from std::make_shared are unnecessary...

... instead std::bad_alloc would be thrown in case of memory exhaustion, which
we don't try to recover from.

* Restore uses of the mayThrow arguments in Sequence...

... that became unused at commit d39590cf41e1e1eac02fc52d88a1ad018824f77b.

It's important to ignore exceptions from SampleBlocks when only displaying, not
editing or playing, and just treat missing data as silence.

Pass the boolean into the SampleBlock routines.  But the throwing of exceptions
is not yet implemented.

* SampleBlockFactory functions guaranteed to return non-null or throw...

... which corrects Sequence.cpp, which was assuming non-null results.

This supplies the throw statements that the previous commit comment says were
still lacking.

This corrects the absence of checks of returns from sql_bind_... function calls
in SqliteSampleBlock.cpp.  (Other calls remain to be checked elsewhere.)

User visible error messages, carried by the exceptions, might be improved.

* Restore the try/catch in AutoSaveFile::Decode...

... which was introduced at 2ba17c78d6758c2f64f2ccfec9af0c537525cea4

but removed at d39590cf41e1e1eac02fc52d88a1ad018824f77b, yet without removing
the throw statement

which left the program vulnerable to abrupt termination instead of graceful
failure, when uninterpretable auto save contents are detected.
This commit is contained in:
Paul Licameli 2020-07-05 15:13:30 -04:00 committed by GitHub
parent c1407cdca9
commit 8b3f9fae58
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 217 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/AutoRecovery.cpp
View File

@ -280,7 +280,7 @@ wxString AutoSaveFile::Decode(const wxMemoryBuffer &buffer)
mIds.clear();
struct Error{};
struct Error{}; // exception type for short-range try/catch
auto Lookup = [&mIds]( short id ) -> const wxString &
{
auto iter = mIds.find( id );
@ -318,7 +318,7 @@ wxString AutoSaveFile::Decode(const wxMemoryBuffer &buffer)
return str;
};
while (!in.Eof())
try { while (!in.Eof())
{
short id;
@ -495,6 +495,10 @@ wxString AutoSaveFile::Decode(const wxMemoryBuffer &buffer)
wxASSERT(true);
break;
}
} } catch( const Error& ) {
// Autosave was corrupt, or platform differences in size or endianness
// were not well canonicalized
return {};
}
return out;

1
src/AutoRecovery.h
View File

@ -61,6 +61,7 @@ public:
bool IsEmpty() const;
bool DictChanged() const;
// Returns empty string if decoding fails
static wxString Decode(const wxMemoryBuffer &buffer);
private:

74
src/SampleBlock.cpp
View File

@ -8,6 +8,7 @@ SampleBlock.cpp
#include "Audacity.h"
#include "SampleBlock.h"
#include "SampleFormat.h"
#include <wx/defs.h>
@ -36,5 +37,78 @@ SampleBlockFactoryPtr SampleBlockFactory::New( AudacityProject &project )
SampleBlockFactory::~SampleBlockFactory() = default;
SampleBlockPtr SampleBlockFactory::Get(SampleBlockID sbid)
{
auto result = DoGet(sbid);
if (!result)
THROW_INCONSISTENCY_EXCEPTION;
return result;
}
SampleBlockPtr SampleBlockFactory::Create(samplePtr src,
size_t numsamples,
sampleFormat srcformat)
{
auto result = DoCreate(src, numsamples, srcformat);
if (!result)
THROW_INCONSISTENCY_EXCEPTION;
return result;
}
SampleBlockPtr SampleBlockFactory::CreateSilent(
size_t numsamples,
sampleFormat srcformat)
{
auto result = DoCreateSilent(numsamples, srcformat);
if (!result)
THROW_INCONSISTENCY_EXCEPTION;
return result;
}
SampleBlockPtr SampleBlockFactory::CreateFromXML(
sampleFormat srcformat,
const wxChar **attrs)
{
auto result = DoCreateFromXML(srcformat, attrs);
if (!result)
THROW_INCONSISTENCY_EXCEPTION;
return result;
}
SampleBlock::~SampleBlock() = default;
size_t SampleBlock::GetSamples(samplePtr dest,
sampleFormat destformat,
size_t sampleoffset,
size_t numsamples, bool mayThrow)
{
try{ return DoGetSamples(dest, destformat, sampleoffset, numsamples); }
catch( ... ) {
if( mayThrow )
throw;
ClearSamples( dest, destformat, 0, numsamples );
return 0;
}
}
MinMaxRMS SampleBlock::GetMinMaxRMS(
size_t start, size_t len, bool mayThrow)
{
try{ return DoGetMinMaxRMS(start, len); }
catch( ... ) {
if( mayThrow )
throw;
return {};
}
}
MinMaxRMS SampleBlock::GetMinMaxRMS(bool mayThrow) const
{
try{ return DoGetMinMaxRMS(); }
catch( ... ) {
if( mayThrow )
throw;
return {};
}
}

66
src/SampleBlock.h
View File

@ -31,9 +31,9 @@ using SampleBlockID = long long;
class MinMaxRMS
{
public:
float min;
float max;
float RMS;
float min = 0;
float max = 0;
float RMS = 0;
};
class SqliteSampleBlockFactory;
@ -52,10 +52,12 @@ public:
virtual SampleBlockID GetBlockID() = 0;
virtual size_t GetSamples(samplePtr dest,
// If !mayThrow and there is an error, ignores it and returns zero.
// That may be appropriate when only attempting to display samples, not edit.
size_t GetSamples(samplePtr dest,
sampleFormat destformat,
size_t sampleoffset,
size_t numsamples) = 0;
size_t numsamples, bool mayThrow = true);
virtual size_t GetSampleCount() const = 0;
@ -65,14 +67,29 @@ public:
GetSummary64k(float *dest, size_t frameoffset, size_t numframes) = 0;
/// Gets extreme values for the specified region
virtual MinMaxRMS GetMinMaxRMS(size_t start, size_t len) = 0;
// If !mayThrow and there is an error, ignores it and returns zeroes.
// That may be appropriate when only attempting to display samples, not edit.
MinMaxRMS GetMinMaxRMS(
size_t start, size_t len, bool mayThrow = true);
/// Gets extreme values for the entire block
virtual MinMaxRMS GetMinMaxRMS() const = 0;
// If !mayThrow and there is an error, ignores it and returns zeroes.
// That may be appropriate when only attempting to display samples, not edit.
MinMaxRMS GetMinMaxRMS(bool mayThrow = true) const;
virtual size_t GetSpaceUsage() const = 0;
virtual void SaveXML(XMLWriter &xmlFile) = 0;
protected:
virtual size_t DoGetSamples(samplePtr dest,
sampleFormat destformat,
size_t sampleoffset,
size_t numsamples) = 0;
virtual MinMaxRMS DoGetMinMaxRMS(size_t start, size_t len) = 0;
virtual MinMaxRMS DoGetMinMaxRMS() const = 0;
};
///\brief abstract base class with methods to produce @ref SampleBlock objects
@ -90,17 +107,44 @@ public:
virtual ~SampleBlockFactory();
virtual SampleBlockPtr Get(SampleBlockID sbid) = 0;
// Returns a non-null pointer or else throws an exception
SampleBlockPtr Get(SampleBlockID sbid);
virtual SampleBlockPtr Create(samplePtr src,
// Returns a non-null pointer or else throws an exception
SampleBlockPtr Create(samplePtr src,
size_t numsamples,
sampleFormat srcformat);
// Returns a non-null pointer or else throws an exception
SampleBlockPtr CreateSilent(
size_t numsamples,
sampleFormat srcformat);
// Returns a non-null pointer or else throws an exception
SampleBlockPtr CreateFromXML(
sampleFormat srcformat,
const wxChar **attrs);
protected:
// The override should throw more informative exceptions on error than the
// default InconsistencyException thrown by Create
virtual SampleBlockPtr DoGet(SampleBlockID sbid) = 0;
// The override should throw more informative exceptions on error than the
// default InconsistencyException thrown by Create
virtual SampleBlockPtr DoCreate(samplePtr src,
size_t numsamples,
sampleFormat srcformat) = 0;
virtual SampleBlockPtr CreateSilent(
// The override should throw more informative exceptions on error than the
// default InconsistencyException thrown by CreateSilent
virtual SampleBlockPtr DoCreateSilent(
size_t numsamples,
sampleFormat srcformat) = 0;
virtual SampleBlockPtr CreateFromXML(
// The override should throw more informative exceptions on error than the
// default InconsistencyException thrown by CreateFromXML
virtual SampleBlockPtr DoCreateFromXML(
sampleFormat srcformat,
const wxChar **attrs) = 0;
};

18
src/Sequence.cpp
View File

@ -255,7 +255,7 @@ std::pair<float, float> Sequence::GetMinMax(
// already in memory.
for (unsigned b = block0 + 1; b < block1; ++b) {
auto results = mBlock[b].sb->GetMinMaxRMS();
auto results = mBlock[b].sb->GetMinMaxRMS(mayThrow);
if (results.min < min)
min = results.min;
@ -270,7 +270,7 @@ std::pair<float, float> Sequence::GetMinMax(
{
const SeqBlock &theBlock = mBlock[block0];
const auto &theFile = theBlock.sb;
auto results = theFile->GetMinMaxRMS();
auto results = theFile->GetMinMaxRMS(mayThrow);
if (results.min < min || results.max > max) {
// start lies within theBlock:
@ -282,7 +282,7 @@ std::pair<float, float> Sequence::GetMinMax(
wxASSERT(maxl0 <= mMaxSamples); // Vaughan, 2011-10-19
const auto l0 = limitSampleBufferSize ( maxl0, len );
results = theFile->GetMinMaxRMS(s0, l0);
results = theFile->GetMinMaxRMS(s0, l0, mayThrow);
if (results.min < min)
min = results.min;
if (results.max > max)
@ -294,7 +294,7 @@ std::pair<float, float> Sequence::GetMinMax(
{
const SeqBlock &theBlock = mBlock[block1];
const auto &theFile = theBlock.sb;
auto results = theFile->GetMinMaxRMS();
auto results = theFile->GetMinMaxRMS(mayThrow);
if (results.min < min || results.max > max) {
@ -302,7 +302,7 @@ std::pair<float, float> Sequence::GetMinMax(
const auto l0 = ( start + len - theBlock.start ).as_size_t();
wxASSERT(l0 <= mMaxSamples); // Vaughan, 2011-10-19
results = theFile->GetMinMaxRMS(0, l0);
results = theFile->GetMinMaxRMS(0, l0, mayThrow);
if (results.min < min)
min = results.min;
if (results.max > max)
@ -332,7 +332,7 @@ float Sequence::GetRMS(sampleCount start, sampleCount len, bool mayThrow) const
for (unsigned b = block0 + 1; b < block1; b++) {
const SeqBlock &theBlock = mBlock[b];
const auto &sb = theBlock.sb;
auto results = sb->GetMinMaxRMS();
auto results = sb->GetMinMaxRMS(mayThrow);
const auto fileLen = sb->GetSampleCount();
const auto blockRMS = results.RMS;
@ -354,7 +354,7 @@ float Sequence::GetRMS(sampleCount start, sampleCount len, bool mayThrow) const
wxASSERT(maxl0 <= mMaxSamples); // Vaughan, 2011-10-19
const auto l0 = limitSampleBufferSize( maxl0, len );
auto results = sb->GetMinMaxRMS(s0, l0);
auto results = sb->GetMinMaxRMS(s0, l0, mayThrow);
const auto partialRMS = results.RMS;
sumsq += partialRMS * partialRMS * l0;
length += l0;
@ -368,7 +368,7 @@ float Sequence::GetRMS(sampleCount start, sampleCount len, bool mayThrow) const
const auto l0 = ( start + len - theBlock.start ).as_size_t();
wxASSERT(l0 <= mMaxSamples); // PRL: I think Vaughan missed this
auto results = sb->GetMinMaxRMS(0, l0);
auto results = sb->GetMinMaxRMS(0, l0, mayThrow);
const auto partialRMS = results.RMS;
sumsq += partialRMS * partialRMS * l0;
length += l0;
@ -1036,7 +1036,7 @@ bool Sequence::Read(samplePtr buffer, sampleFormat format,
wxASSERT(blockRelativeStart + len <= sb->GetSampleCount());
// Either throws, or of !mayThrow, tells how many were really read
auto result = sb->GetSamples(buffer, format, blockRelativeStart, len);
auto result = sb->GetSamples(buffer, format, blockRelativeStart, len, mayThrow);
if (result != len)
{

142
src/SqliteSampleBlock.cpp
View File

@ -29,17 +29,17 @@ public:
void Unlock() override;
void CloseLock() override;
bool SetSamples(samplePtr src, size_t numsamples, sampleFormat srcformat);
void SetSamples(samplePtr src, size_t numsamples, sampleFormat srcformat);
bool SetSilent(size_t numsamples, sampleFormat srcformat);
void SetSilent(size_t numsamples, sampleFormat srcformat);
bool Commit();
void Commit();
void Delete();
SampleBlockID GetBlockID() override;
size_t GetSamples(samplePtr dest,
size_t DoGetSamples(samplePtr dest,
sampleFormat destformat,
size_t sampleoffset,
size_t numsamples) override;
@ -53,10 +53,10 @@ public:
double GetSumRms() const;
/// Gets extreme values for the specified region
MinMaxRMS GetMinMaxRMS(size_t start, size_t len) override;
MinMaxRMS DoGetMinMaxRMS(size_t start, size_t len) override;
/// Gets extreme values for the entire block
MinMaxRMS GetMinMaxRMS() const override;
MinMaxRMS DoGetMinMaxRMS() const override;
size_t GetSpaceUsage() const override;
void SaveXML(XMLWriter &xmlFile) override;
@ -116,17 +116,17 @@ public:
~SqliteSampleBlockFactory() override;
SampleBlockPtr Get(SampleBlockID sbid) override;
SampleBlockPtr DoGet(SampleBlockID sbid) override;
SampleBlockPtr Create(samplePtr src,
SampleBlockPtr DoCreate(samplePtr src,
size_t numsamples,
sampleFormat srcformat) override;
SampleBlockPtr CreateSilent(
SampleBlockPtr DoCreateSilent(
size_t numsamples,
sampleFormat srcformat) override;
SampleBlockPtr CreateFromXML(
SampleBlockPtr DoCreateFromXML(
sampleFormat srcformat,
const wxChar **attrs) override;
@ -144,40 +144,24 @@ SqliteSampleBlockFactory::SqliteSampleBlockFactory( AudacityProject &project )
SqliteSampleBlockFactory::~SqliteSampleBlockFactory() = default;
SampleBlockPtr SqliteSampleBlockFactory::Create(
SampleBlockPtr SqliteSampleBlockFactory::DoCreate(
samplePtr src, size_t numsamples, sampleFormat srcformat )
{
auto sb = std::make_shared<SqliteSampleBlock>(&mProject);
if (sb)
{
if (sb->SetSamples(src, numsamples, srcformat))
{
sb->SetSamples(src, numsamples, srcformat);
return sb;
}
}
return nullptr;
}
SampleBlockPtr SqliteSampleBlockFactory::CreateSilent(
SampleBlockPtr SqliteSampleBlockFactory::DoCreateSilent(
size_t numsamples, sampleFormat srcformat )
{
auto sb = std::make_shared<SqliteSampleBlock>(&mProject);
if (sb)
{
if (sb->SetSilent(numsamples, srcformat))
{
sb->SetSilent(numsamples, srcformat);
return sb;
}
}
return nullptr;
}
SampleBlockPtr SqliteSampleBlockFactory::CreateFromXML(
SampleBlockPtr SqliteSampleBlockFactory::DoCreateFromXML(
sampleFormat srcformat, const wxChar **attrs )
{
auto sb = std::make_shared<SqliteSampleBlock>(&mProject);
@ -204,10 +188,8 @@ SampleBlockPtr SqliteSampleBlockFactory::CreateFromXML(
{
if (wxStrcmp(attr, wxT("blockid")) == 0)
{
if (!sb->Load((SampleBlockID) nValue))
{
return nullptr;
}
// This may throw
sb->Load((SampleBlockID) nValue);
found++;
}
else if (wxStrcmp(attr, wxT("samplecount")) == 0)
@ -246,18 +228,10 @@ SampleBlockPtr SqliteSampleBlockFactory::CreateFromXML(
return sb;
}
SampleBlockPtr SqliteSampleBlockFactory::Get( SampleBlockID sbid )
SampleBlockPtr SqliteSampleBlockFactory::DoGet( SampleBlockID sbid )
{
auto sb = std::make_shared<SqliteSampleBlock>(&mProject);
if (sb)
{
if (!sb->Load(sbid))
{
return nullptr;
}
}
sb->Load(sbid);
return sb;
}
@ -286,7 +260,12 @@ SqliteSampleBlock::~SqliteSampleBlock()
// See ProjectFileIO::Bypass() for a description of mIO.mBypass
if (mRefCnt == 0 && !mIO.ShouldBypass())
{
Delete();
// In case Delete throws, don't let an exception escape a destructor,
// but we can still enqueue the delayed handler so that an error message
// is presented to the user.
// The failure in this case may be a less harmful waste of space in the
// database, which should not cause aborting of the attempted edit.
GuardedCall( [this]{ Delete(); } );
}
}
@ -320,7 +299,7 @@ size_t SqliteSampleBlock::GetSampleCount() const
return mSampleCount;
}
size_t SqliteSampleBlock::GetSamples(samplePtr dest,
size_t SqliteSampleBlock::DoGetSamples(samplePtr dest,
sampleFormat destformat,
size_t sampleoffset,
size_t numsamples)
@ -333,7 +312,7 @@ size_t SqliteSampleBlock::GetSamples(samplePtr dest,
numsamples * SAMPLE_SIZE(mSampleFormat)) / SAMPLE_SIZE(mSampleFormat);
}
bool SqliteSampleBlock::SetSamples(samplePtr src,
void SqliteSampleBlock::SetSamples(samplePtr src,
size_t numsamples,
sampleFormat srcformat)
{
@ -346,10 +325,10 @@ bool SqliteSampleBlock::SetSamples(samplePtr src,
CalcSummary();
return Commit();
Commit();
}
bool SqliteSampleBlock::SetSilent(size_t numsamples, sampleFormat srcformat)
void SqliteSampleBlock::SetSilent(size_t numsamples, sampleFormat srcformat)
{
mSampleFormat = srcformat;
@ -362,7 +341,7 @@ bool SqliteSampleBlock::SetSilent(size_t numsamples, sampleFormat srcformat)
mSilent = true;
return Commit();
Commit();
}
bool SqliteSampleBlock::GetSummary256(float *dest,
@ -413,7 +392,7 @@ double SqliteSampleBlock::GetSumRms() const
///
/// @param start The offset in this block where the region should begin
/// @param len The number of samples to include in the region
MinMaxRMS SqliteSampleBlock::GetMinMaxRMS(size_t start, size_t len)
MinMaxRMS SqliteSampleBlock::DoGetMinMaxRMS(size_t start, size_t len)
{
float min = FLT_MAX;
float max = -FLT_MAX;
@ -462,7 +441,7 @@ MinMaxRMS SqliteSampleBlock::GetMinMaxRMS(size_t start, size_t len)
/// Retrieves the minimum, maximum, and maximum RMS of this entire
/// block. This is faster than the other GetMinMax function since
/// these values are already computed.
MinMaxRMS SqliteSampleBlock::GetMinMaxRMS() const
MinMaxRMS SqliteSampleBlock::DoGetMinMaxRMS() const
{
return { (float) mSumMin, (float) mSumMax, (float) mSumRms };
}
@ -541,6 +520,11 @@ size_t SqliteSampleBlock::GetBlob(void *dest,
}
}
if ( rc != SQLITE_ROW )
// Just showing the user a simple message, not the library error too
// which isn't internationalized
throw SimpleMessageBoxException{ XO("Failed to retrieve samples") };
if (srcbytes - minbytes)
{
memset(dest, 0, srcbytes - minbytes);
@ -587,17 +571,19 @@ bool SqliteSampleBlock::Load(SampleBlockID sbid)
if (rc != SQLITE_OK)
{
wxLogDebug(wxT("SQLITE error %s"), sqlite3_errmsg(db));
// handle error
return false;
}
else {
rc = sqlite3_step(stmt);
if (rc != SQLITE_ROW)
{
wxLogDebug(wxT("SQLITE error %s"), sqlite3_errmsg(db));
// handle error
return false;
}
}
if ( rc != SQLITE_ROW )
// Just showing the user a simple message, not the library error too
// which isn't internationalized
throw SimpleMessageBoxException{ XO("Failed to retrieve samples") };
mBlockID = sbid;
mSampleFormat = (sampleFormat) sqlite3_column_int(stmt, 0);
@ -610,11 +596,9 @@ bool SqliteSampleBlock::Load(SampleBlockID sbid)
mSampleCount = mSampleBytes / SAMPLE_SIZE(mSampleFormat);
mValid = true;
return true;
}
bool SqliteSampleBlock::Commit()
void SqliteSampleBlock::Commit()
{
auto db = mIO.DB();
int rc;
@ -638,25 +622,32 @@ bool SqliteSampleBlock::Commit()
if (rc != SQLITE_OK)
{
wxLogDebug(wxT("SQLITE error %s"), sqlite3_errmsg(db));
// handle error
return false;
// Just showing the user a simple message, not the library error too
// which isn't internationalized
throw SimpleMessageBoxException{ mIO.GetLastError() };
}
// BIND SQL sampleblocks
sqlite3_bind_int(stmt, 1, mSampleFormat);
sqlite3_bind_double(stmt, 2, mSumMin);
sqlite3_bind_double(stmt, 3, mSumMax);
sqlite3_bind_double(stmt, 4, mSumRms);
sqlite3_bind_blob(stmt, 5, mSummary256.get(), mSummary256Bytes, SQLITE_STATIC);
sqlite3_bind_blob(stmt, 6, mSummary64k.get(), mSummary64kBytes, SQLITE_STATIC);
sqlite3_bind_blob(stmt, 7, mSamples.get(), mSampleBytes, SQLITE_STATIC);
// Might return SQL_MISUSE which means it's our mistake that we violated
// preconditions; should return SQL_OK which is 0
if (
sqlite3_bind_int(stmt, 1, mSampleFormat) ||
sqlite3_bind_double(stmt, 2, mSumMin) ||
sqlite3_bind_double(stmt, 3, mSumMax) ||
sqlite3_bind_double(stmt, 4, mSumRms) ||
sqlite3_bind_blob(stmt, 5, mSummary256.get(), mSummary256Bytes, SQLITE_STATIC) ||
sqlite3_bind_blob(stmt, 6, mSummary64k.get(), mSummary64kBytes, SQLITE_STATIC) ||
sqlite3_bind_blob(stmt, 7, mSamples.get(), mSampleBytes, SQLITE_STATIC)
)
THROW_INCONSISTENCY_EXCEPTION;
rc = sqlite3_step(stmt);
if (rc != SQLITE_DONE)
{
wxLogDebug(wxT("SQLITE error %s"), sqlite3_errmsg(db));
// handle error
return false;
// Just showing the user a simple message, not the library error too
// which isn't internationalized
throw SimpleMessageBoxException{ mIO.GetLastError() };
}
mBlockID = sqlite3_last_insert_rowid(db);
@ -666,8 +657,6 @@ bool SqliteSampleBlock::Commit()
mSummary64k.reset();
mValid = true;
return true;
}
void SqliteSampleBlock::Delete()
@ -688,8 +677,9 @@ void SqliteSampleBlock::Delete()
if (rc != SQLITE_OK)
{
wxLogDebug(wxT("SQLITE error %s"), sqlite3_errmsg(db));
// handle error
return;
// Just showing the user a simple message, not the library error too
// which isn't internationalized
throw SimpleMessageBoxException{ XO("Failed to purge unused samples") };
}
}
}