Skip to content

Commit edb0614

Browse files
authored
[CORE] Fix unsafe logic in FileSystem regarding closing and deleting files (#1303)
1 parent 143673a commit edb0614

File tree

9 files changed

+65
-42
lines changed

9 files changed

+65
-42
lines changed

Core/GameEngine/Include/Common/LocalFile.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,10 @@ class LocalFile : public File
115115
virtual char* readEntireAndClose();
116116
virtual File* convertToRAMFile();
117117

118+
protected:
119+
120+
void closeWithoutDelete();
121+
void closeFile();
118122
};
119123

120124

Core/GameEngine/Include/Common/RAMFile.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,10 @@ class RAMFile : public File
111111
*/
112112
virtual char* readEntireAndClose();
113113
virtual File* convertToRAMFile();
114+
115+
protected:
116+
117+
void closeFile();
114118
};
115119

116120

Core/GameEngine/Include/Common/file.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ class File : public MemoryPoolObject
122122
File(); ///< This class can only used as a base class
123123
//virtual ~File();
124124

125+
void closeWithoutDelete();
126+
125127
public:
126128

127129

Core/GameEngine/Source/Common/System/File.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ File::File()
122122

123123
File::~File()
124124
{
125-
close();
126125
}
127126

128127
//=================================================================
@@ -197,6 +196,17 @@ void File::close( void )
197196
}
198197
}
199198

199+
//=================================================================
200+
201+
void File::closeWithoutDelete()
202+
{
203+
if( m_open )
204+
{
205+
setName( "<no file>" );
206+
m_open = FALSE;
207+
}
208+
}
209+
200210
//=================================================================
201211
// File::size
202212
//=================================================================

Core/GameEngine/Source/Common/System/LocalFile.cpp

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -127,24 +127,7 @@ LocalFile::LocalFile()
127127

128128
LocalFile::~LocalFile()
129129
{
130-
#if USE_BUFFERED_IO
131-
if (m_file)
132-
{
133-
fclose(m_file);
134-
m_file = NULL;
135-
--s_totalOpen;
136-
}
137-
#else
138-
if( m_handle != -1 )
139-
{
140-
_close( m_handle );
141-
m_handle = -1;
142-
--s_totalOpen;
143-
}
144-
#endif
145-
146-
File::close();
147-
130+
closeFile();
148131
}
149132

150133
//=================================================================
@@ -278,7 +261,8 @@ Bool LocalFile::open( const Char *filename, Int access )
278261

279262
error:
280263

281-
close();
264+
// TheSuperHackers @fix Instance must never delete itself in this function.
265+
closeWithoutDelete();
282266

283267
return FALSE;
284268
}
@@ -294,9 +278,39 @@ Bool LocalFile::open( const Char *filename, Int access )
294278

295279
void LocalFile::close( void )
296280
{
281+
closeFile();
297282
File::close();
298283
}
299284

285+
//=================================================================
286+
287+
void LocalFile::closeWithoutDelete()
288+
{
289+
closeFile();
290+
File::closeWithoutDelete();
291+
}
292+
293+
//=================================================================
294+
295+
void LocalFile::closeFile()
296+
{
297+
#if USE_BUFFERED_IO
298+
if (m_file)
299+
{
300+
fclose(m_file);
301+
m_file = NULL;
302+
--s_totalOpen;
303+
}
304+
#else
305+
if( m_handle != -1 )
306+
{
307+
_close( m_handle );
308+
m_handle = -1;
309+
--s_totalOpen;
310+
}
311+
#endif
312+
}
313+
300314
//=================================================================
301315
// LocalFile::read
302316
//=================================================================
@@ -575,18 +589,12 @@ File* LocalFile::convertToRAMFile()
575589
if (this->m_deleteOnClose)
576590
{
577591
ramFile->deleteOnClose();
578-
this->close(); // is deleteonclose, so should delete.
579-
}
580-
else
581-
{
582-
this->close();
583-
deleteInstance(this);
584592
}
593+
deleteInstance(this);
585594
return ramFile;
586595
}
587596
else
588597
{
589-
ramFile->close();
590598
deleteInstance(ramFile);
591599
return this;
592600
}

Core/GameEngine/Source/Common/System/RAMFile.cpp

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,7 @@ RAMFile::RAMFile()
126126

127127
RAMFile::~RAMFile()
128128
{
129-
if (m_data != NULL) {
130-
delete [] m_data;
131-
}
132-
133-
File::close();
134-
129+
closeFile();
135130
}
136131

137132
//=================================================================
@@ -247,15 +242,18 @@ Bool RAMFile::openFromArchive(File *archiveFile, const AsciiString& filename, In
247242

248243
void RAMFile::close( void )
249244
{
250-
if ( m_data )
251-
{
252-
delete [] m_data;
253-
m_data = NULL;
254-
}
255-
245+
closeFile();
256246
File::close();
257247
}
258248

249+
//=================================================================
250+
251+
void RAMFile::closeFile()
252+
{
253+
delete [] m_data;
254+
m_data = NULL;
255+
}
256+
259257
//=================================================================
260258
// RAMFile::read
261259
//=================================================================

Core/GameEngine/Source/Common/System/StreamingArchiveFile.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,6 @@ StreamingArchiveFile::StreamingArchiveFile()
124124

125125
StreamingArchiveFile::~StreamingArchiveFile()
126126
{
127-
File::close();
128127
}
129128

130129
//=================================================================

Core/GameEngineDevice/Source/StdDevice/Common/StdLocalFileSystem.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,6 @@ File * StdLocalFileSystem::openFile(const Char *filename, Int access /* = 0 */)
156156
StdLocalFile *file = newInstance( StdLocalFile );
157157

158158
if (file->open(path.string().c_str(), access) == FALSE) {
159-
file->close();
160159
deleteInstance(file);
161160
file = NULL;
162161
} else {

Core/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ File * Win32LocalFileSystem::openFile(const Char *filename, Int access /* = 0 */
7272
Win32LocalFile *file = newInstance( Win32LocalFile );
7373

7474
if (file->open(filename, access) == FALSE) {
75-
file->close();
7675
deleteInstance(file);
7776
file = NULL;
7877
} else {

0 commit comments

Comments
 (0)