1
0
mirror of https://github.com/cookiengineer/audacity synced 2025-06-16 16:10:06 +02:00

Review of ImportAup.cpp ...

... incidental to a review of CopySamples in it.  This code is used only when
importing an old-style .aup project file into the new project format.

Fixed some RAII for file handle.

Rewrote every call to IsGoodInt or IsGoodInt64 with a narrowly scoped temporary
variable.

Use IsGoodInt64 only for total track lengths and positions; but use IsGoodInt
for block and blockfile sizes, which are not supposed to be huge but are
supposed to fit in memory buffers.
This commit is contained in:
Paul Licameli 2021-05-22 14:25:53 -04:00
parent 0053c61c08
commit 43310b6a1e

View File

@ -221,6 +221,10 @@ private:
AUPImportPlugin::AUPImportPlugin()
: ImportPlugin(FileExtensions(exts.begin(), exts.end()))
{
static_assert(
sizeof(long long) >= sizeof(uint64_t) &&
sizeof(long) >= sizeof(uint32_t),
"Assumptions about sizes in XMLValueChecker calls are invalid!");
}
AUPImportPlugin::~AUPImportPlugin()
@ -643,8 +647,6 @@ bool AUPImportFileHandle::HandleProject(XMLTagHandler *&handler)
{
const wxChar *attr = *mAttrs++;
const wxChar *value = *mAttrs++;
long lValue;
long long llValue;
double dValue;
if (!value)
@ -664,6 +666,7 @@ bool AUPImportFileHandle::HandleProject(XMLTagHandler *&handler)
// ViewInfo
if (!wxStrcmp(attr, wxT("vpos")))
{
long lValue;
if (!XMLValueChecker::IsGoodInt(strValue) || !strValue.ToLong(&lValue) || (lValue < 0))
{
return SetError(XO("Invalid project 'vpos' attribute."));
@ -1115,14 +1118,13 @@ bool AUPImportFileHandle::HandleSequence(XMLTagHandler *&handler)
break;
}
long long nValue = 0;
const wxString strValue = value; // promote string, we need this for all
if (!wxStrcmp(attr, wxT("maxsamples")))
{
// This attribute is a sample count, so can be 64bit
if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&nValue) || (nValue < 0))
long long llvalue;
if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&llvalue) || (llvalue < 0))
{
return SetError(XO("Invalid sequence 'maxsamples' attribute."));
}
@ -1130,7 +1132,7 @@ bool AUPImportFileHandle::HandleSequence(XMLTagHandler *&handler)
// Dominic, 12/10/2006:
// Let's check that maxsamples is >= 1024 and <= 64 * 1024 * 1024
// - that's a pretty wide range of reasonable values.
if ((nValue < 1024) || (nValue > 64 * 1024 * 1024))
if ((llvalue < 1024) || (llvalue > 64 * 1024 * 1024))
{
return SetError(XO("Invalid sequence 'maxsamples' attribute."));
}
@ -1150,7 +1152,8 @@ bool AUPImportFileHandle::HandleSequence(XMLTagHandler *&handler)
else if (!wxStrcmp(attr, wxT("numsamples")))
{
// This attribute is a sample count, so can be 64bit
if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&nValue) || (nValue < 0))
long long llvalue;
if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&llvalue) || (llvalue < 0))
{
return SetError(XO("Invalid sequence 'numsamples' attribute."));
}
@ -1169,8 +1172,6 @@ bool AUPImportFileHandle::HandleWaveBlock(XMLTagHandler *&handler)
const wxChar *attr = *mAttrs++;
const wxChar *value = *mAttrs++;
long long nValue = 0;
if (!value)
{
break;
@ -1181,7 +1182,8 @@ bool AUPImportFileHandle::HandleWaveBlock(XMLTagHandler *&handler)
if (!wxStrcmp(attr, wxT("start")))
{
// making sure that values > 2^31 are OK because long clips will need them.
if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&nValue) || (nValue < 0))
long long llvalue;
if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&llvalue) || (llvalue < 0))
{
return SetError(XO("Unable to parse the waveblock 'start' attribute"));
}
@ -1196,13 +1198,12 @@ bool AUPImportFileHandle::HandleWaveBlock(XMLTagHandler *&handler)
bool AUPImportFileHandle::HandleSimpleBlockFile(XMLTagHandler *&handler)
{
FilePath filename;
sampleCount len = 0;
size_t len = 0;
while (*mAttrs)
{
const wxChar *attr = *mAttrs++;
const wxChar *value = *mAttrs++;
long long nValue;
if (!value)
{
@ -1229,12 +1230,13 @@ bool AUPImportFileHandle::HandleSimpleBlockFile(XMLTagHandler *&handler)
}
else if (!wxStrcmp(attr, wxT("len")))
{
if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&nValue) || (nValue <= 0))
long lValue;
if (!XMLValueChecker::IsGoodInt(strValue) || !strValue.ToLong(&lValue) || (lValue <= 0))
{
return SetError(XO("Missing or invalid simpleblockfile 'len' attribute."));
}
len = nValue;
len = lValue;
}
}
@ -1248,13 +1250,12 @@ bool AUPImportFileHandle::HandleSimpleBlockFile(XMLTagHandler *&handler)
bool AUPImportFileHandle::HandleSilentBlockFile(XMLTagHandler *&handler)
{
FilePath filename;
sampleCount len = 0;
size_t len = 0;
while (*mAttrs)
{
const wxChar *attr = *mAttrs++;
const wxChar *value = *mAttrs++;
long long nValue;
if (!value)
{
@ -1265,12 +1266,13 @@ bool AUPImportFileHandle::HandleSilentBlockFile(XMLTagHandler *&handler)
if (!wxStrcmp(attr, wxT("len")))
{
if (!XMLValueChecker::IsGoodInt64(value) || !strValue.ToLongLong(&nValue) || !(nValue > 0))
long lValue;
if (!XMLValueChecker::IsGoodInt(value) || !strValue.ToLong(&lValue) || !(lValue > 0))
{
return SetError(XO("Missing or invalid silentblockfile 'len' attribute."));
}
len = nValue;
len = lValue;
}
}
@ -1286,7 +1288,7 @@ bool AUPImportFileHandle::HandlePCMAliasBlockFile(XMLTagHandler *&handler)
wxString summaryFilename;
wxFileName filename;
sampleCount start = 0;
sampleCount len = 0;
size_t len = 0;
int channel = 0;
wxString name;
@ -1294,7 +1296,6 @@ bool AUPImportFileHandle::HandlePCMAliasBlockFile(XMLTagHandler *&handler)
{
const wxChar *attr = *mAttrs++;
const wxChar *value = *mAttrs++;
long long nValue;
if (!value)
{
@ -1328,31 +1329,33 @@ bool AUPImportFileHandle::HandlePCMAliasBlockFile(XMLTagHandler *&handler)
}
else if (!wxStricmp(attr, wxT("aliasstart")))
{
if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&nValue) || (nValue < 0))
long long llValue;
if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&llValue) || (llValue < 0))
{
return SetError(XO("Missing or invalid pcmaliasblockfile 'aliasstart' attribute."));
}
start = nValue;
start = llValue;
}
else if (!wxStricmp(attr, wxT("aliaslen")))
{
if (!XMLValueChecker::IsGoodInt64(strValue) || !strValue.ToLongLong(&nValue) || (nValue <= 0))
long lValue;
if (!XMLValueChecker::IsGoodInt(strValue) || !strValue.ToLong(&lValue) || (lValue <= 0))
{
return SetError(XO("Missing or invalid pcmaliasblockfile 'aliaslen' attribute."));
}
len = nValue;
len = lValue;
}
else if (!wxStricmp(attr, wxT("aliaschannel")))
{
long nValue;
if (!XMLValueChecker::IsGoodInt(strValue) || !strValue.ToLong(&nValue) || (nValue < 0))
long lValue;
if (!XMLValueChecker::IsGoodInt(strValue) || !strValue.ToLong(&lValue) || (lValue < 0))
{
return SetError(XO("Missing or invalid pcmaliasblockfile 'aliaslen' attribute."));
}
channel = nValue;
channel = lValue;
}
}
@ -1512,6 +1515,12 @@ bool AUPImportFileHandle::AddSamples(const FilePath &blockFilename,
auto cleanup = finally([&]
{
// Do this before any throwing might happen
if (sf)
{
SFCall<int>(sf_close, sf);
}
if (!success)
{
SetWarning(XO("Error while processing %s\n\nInserting silence.").Format(audioFilename));
@ -1522,11 +1531,6 @@ bool AUPImportFileHandle::AddSamples(const FilePath &blockFilename,
// If this does throw, let that propagate, don't guard the call
AddSilence(len);
}
if (sf)
{
SFCall<int>(sf_close, sf);
}
});
if (!f.Open(audioFilename))