Skip to content

Commit

Permalink
fix double delete in multipart files, check logic in others
Browse files Browse the repository at this point in the history 
Multipart files have a Data object that automatically cleans up it's
stream if appropriate, the other file objects have the destructor of the
file object perform the delete (instead of Data). This causes a double
delete to happen in MultiPart objects when unable to open a stream.

Additionally, fix tabs / spaces to just be spaces

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
  • Loading branch information
@kdt3rd @nickrasmussen
kdt3rd authored and nickrasmussen committed Aug 8, 2018
1 parent c246315 commit da96e37
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 60 deletions.
2 changes: 1 addition & 1 deletion OpenEXR/IlmImf/ImfDeepScanLineOutputFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,7 @@ DeepScanLineOutputFile::DeepScanLineOutputFile
_data->header.writeTo (*_data->_streamData->os);
_data->lineOffsetsPosition =
writeLineOffsets (*_data->_streamData->os, _data->lineOffsets);
_data->multipart=false;// not multipart; only one header
_data->multipart=false;// not multipart; only one header
}
catch (IEX_NAMESPACE::BaseExc &e)
{
Expand Down
2 changes: 0 additions & 2 deletions OpenEXR/IlmImf/ImfMultiPartInputFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ MultiPartInputFile::MultiPartInputFile(const char fileName[],
}
catch (IEX_NAMESPACE::BaseExc &e)
{
delete _data->is;
delete _data;

REPLACE_EXC (e, "Cannot read image file "
Expand All @@ -146,7 +145,6 @@ MultiPartInputFile::MultiPartInputFile(const char fileName[],
}
catch (...)
{
delete _data->is;
delete _data;
throw;
}
Expand Down
2 changes: 0 additions & 2 deletions OpenEXR/IlmImf/ImfMultiPartOutputFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ MultiPartOutputFile::MultiPartOutputFile (const char fileName[],
}
catch (IEX_NAMESPACE::BaseExc &e)
{
delete _data->os;
delete _data;

REPLACE_EXC (e, "Cannot open image file "
Expand All @@ -235,7 +234,6 @@ MultiPartOutputFile::MultiPartOutputFile (const char fileName[],
}
catch (...)
{
delete _data->os;
delete _data;
throw;
}
Expand Down
64 changes: 33 additions & 31 deletions OpenEXR/IlmImf/ImfOutputFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -657,40 +657,41 @@ OutputFile::OutputFile
int numThreads)
:
_data (new Data (numThreads))

{
_data->_streamData=new OutputStreamMutex ();
_data->_deleteStream=true;
try
{
header.sanityCheck();
_data->_streamData->os = new StdOFStream (fileName);
header.sanityCheck();
_data->_streamData->os = new StdOFStream (fileName);
_data->multiPart=false; // only one header, not multipart
initialize (header);
_data->_streamData->currentPosition = _data->_streamData->os->tellp();
initialize (header);
_data->_streamData->currentPosition = _data->_streamData->os->tellp();

// Write header and empty offset table to the file.
writeMagicNumberAndVersionField(*_data->_streamData->os, _data->header);
_data->previewPosition =
_data->header.writeTo (*_data->_streamData->os);
// Write header and empty offset table to the file.
writeMagicNumberAndVersionField(*_data->_streamData->os, _data->header);
_data->previewPosition =
_data->header.writeTo (*_data->_streamData->os);
_data->lineOffsetsPosition =
writeLineOffsets (*_data->_streamData->os,_data->lineOffsets);
writeLineOffsets (*_data->_streamData->os,_data->lineOffsets);
}
catch (IEX_NAMESPACE::BaseExc &e)
{
// ~OutputFile will not run, so free memory here
delete _data->_streamData->os;
delete _data->_streamData;
delete _data;

REPLACE_EXC (e, "Cannot open image file "
"\"" << fileName << "\". " << e.what());
throw;
REPLACE_EXC (e, "Cannot open image file "
"\"" << fileName << "\". " << e.what());
throw;
}
catch (...)
{
// ~OutputFile will not run, so free memory here
delete _data->_streamData->os;
delete _data->_streamData;
delete _data;
delete _data;

throw;
}
Expand All @@ -704,37 +705,38 @@ OutputFile::OutputFile
:
_data (new Data (numThreads))
{

_data->_streamData=new OutputStreamMutex ();
_data->_deleteStream=false;
try
{
header.sanityCheck();
_data->_streamData->os = &os;
header.sanityCheck();
_data->_streamData->os = &os;
_data->multiPart=false;
initialize (header);
_data->_streamData->currentPosition = _data->_streamData->os->tellp();
initialize (header);
_data->_streamData->currentPosition = _data->_streamData->os->tellp();

// Write header and empty offset table to the file.
writeMagicNumberAndVersionField(*_data->_streamData->os, _data->header);
_data->previewPosition =
_data->header.writeTo (*_data->_streamData->os);
// Write header and empty offset table to the file.
writeMagicNumberAndVersionField(*_data->_streamData->os, _data->header);
_data->previewPosition =
_data->header.writeTo (*_data->_streamData->os);
_data->lineOffsetsPosition =
writeLineOffsets (*_data->_streamData->os, _data->lineOffsets);
writeLineOffsets (*_data->_streamData->os, _data->lineOffsets);
}
catch (IEX_NAMESPACE::BaseExc &e)
{
if (_data && _data->_streamData) delete _data->_streamData;
if (_data) delete _data;
// ~OutputFile will not run, so free memory here
delete _data->_streamData;
delete _data;

REPLACE_EXC (e, "Cannot open image file "
"\"" << os.fileName() << "\". " << e.what());
throw;
REPLACE_EXC (e, "Cannot open image file "
"\"" << os.fileName() << "\". " << e.what());
throw;
}
catch (...)
{
if (_data && _data->_streamData) delete _data->_streamData;
if (_data) delete _data;
// ~OutputFile will not run, so free memory here
delete _data->_streamData;
delete _data;

throw;
}
Expand Down
50 changes: 26 additions & 24 deletions OpenEXR/IlmImf/ImfTiledOutputFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -863,32 +863,34 @@ TiledOutputFile::TiledOutputFile
{
try
{
header.sanityCheck (true);
_streamData->os = new StdOFStream (fileName);
header.sanityCheck (true);
_streamData->os = new StdOFStream (fileName);
_data->multipart=false; // since we opened with one header we can't be multipart
initialize (header);
_streamData->currentPosition = _streamData->os->tellp();
initialize (header);
_streamData->currentPosition = _streamData->os->tellp();

// Write header and empty offset table to the file.
// Write header and empty offset table to the file.
writeMagicNumberAndVersionField(*_streamData->os, _data->header);
_data->previewPosition = _data->header.writeTo (*_streamData->os, true);
_data->previewPosition = _data->header.writeTo (*_streamData->os, true);
_data->tileOffsetsPosition = _data->tileOffsets.writeTo (*_streamData->os);
}
catch (IEX_NAMESPACE::BaseExc &e)
{
// ~TiledOutputFile will not run, so free memory here
delete _streamData->os;
delete _streamData;
delete _data;
delete _data;

REPLACE_EXC (e, "Cannot open image file "
"\"" << fileName << "\". " << e.what());
throw;
REPLACE_EXC (e, "Cannot open image file "
"\"" << fileName << "\". " << e.what());
throw;
}
catch (...)
{
// ~TiledOutputFile will not run, so free memory here
delete _streamData->os;
delete _streamData;
delete _data;
delete _data;
throw;
}
}
Expand All @@ -905,31 +907,31 @@ TiledOutputFile::TiledOutputFile
{
try
{
header.sanityCheck(true);
_streamData->os = &os;
header.sanityCheck(true);
_streamData->os = &os;
_data->multipart=false; // since we opened with one header we can't be multipart
initialize (header);
_streamData->currentPosition = _streamData->os->tellp();
initialize (header);
_streamData->currentPosition = _streamData->os->tellp();

// Write header and empty offset table to the file.
writeMagicNumberAndVersionField(*_streamData->os, _data->header);
_data->previewPosition = _data->header.writeTo (*_streamData->os, true);
// Write header and empty offset table to the file.
writeMagicNumberAndVersionField(*_streamData->os, _data->header);
_data->previewPosition = _data->header.writeTo (*_streamData->os, true);
_data->tileOffsetsPosition = _data->tileOffsets.writeTo (*_streamData->os);
}
catch (IEX_NAMESPACE::BaseExc &e)
{
delete _streamData;
delete _data;
delete _data;

REPLACE_EXC (e, "Cannot open image file "
"\"" << os.fileName() << "\". " << e.what());
throw;
REPLACE_EXC (e, "Cannot open image file "
"\"" << os.fileName() << "\". " << e.what());
throw;
}
catch (...)
{
delete _streamData;
delete _data;
delete _data;
throw;
}
}
Expand Down

0 comments on commit da96e37

Please sign in to comment.