diff --git a/docs/changes.txt b/docs/changes.txt index e53e0cd8df..9504fc7e02 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -84,6 +84,7 @@ All: wxEvtHandler and/or wxTrackable in C++11 code (Raul Tambre, mmarsan). - Update bundled expat to 2.2.0 (Catalin Raceanu). - Add wxCMD_LINE_HIDDEN wxCmdLineParser flag (Lauri Nurmi). +- Don't crash in wxFFile::Eof() or Error() on closed file (jprotopopov). All (GUI): diff --git a/include/wx/ffile.h b/include/wx/ffile.h index 91a7c28ed4..98041fcac1 100644 --- a/include/wx/ffile.h +++ b/include/wx/ffile.h @@ -76,13 +76,13 @@ public: wxFileOffset Length() const; // simple accessors: note that Eof() and Error() may only be called if - // IsOpened()! + // IsOpened(). Otherwise they assert and return false. // is file opened? bool IsOpened() const { return m_fp != NULL; } // is end of file reached? - bool Eof() const { return feof(m_fp) != 0; } + bool Eof() const; // has an error occurred? - bool Error() const { return ferror(m_fp) != 0; } + bool Error() const; // get the file name const wxString& GetName() const { return m_name; } // type such as disk or pipe diff --git a/interface/wx/ffile.h b/interface/wx/ffile.h index ada146d343..90cc9d652b 100644 --- a/interface/wx/ffile.h +++ b/interface/wx/ffile.h @@ -89,10 +89,7 @@ public: Note that the behaviour of the file descriptor based class wxFile is different as wxFile::Eof() will return @true here as soon as the last byte of the file has been read. - Also note that this method may only be called for opened files and may crash if - the file is not opened. - - @todo THIS METHOD MAY CRASH? DOESN'T SOUND GOOD + Also note that this method may only be called for opened files. Otherwise it asserts and returns false. @see IsOpened() */ @@ -102,10 +99,7 @@ public: Returns @true if an error has occurred on this file, similar to the standard @c ferror() function. - Please note that this method may only be called for opened files and may crash - if the file is not opened. - - @todo THIS METHOD MAY CRASH? DOESN'T SOUND GOOD + Please note that this method may only be called for opened files. Otherwise it asserts and returns false. @see IsOpened() */ diff --git a/src/common/ffile.cpp b/src/common/ffile.cpp index 1f729c2cc2..ede1e9a157 100644 --- a/src/common/ffile.cpp +++ b/src/common/ffile.cpp @@ -281,4 +281,18 @@ wxFileOffset wxFFile::Length() const return wxInvalidOffset; } +bool wxFFile::Eof() const +{ + wxCHECK_MSG( IsOpened(), false, + wxT("wxFFile::Eof(): file is closed!") ); + return feof(m_fp) != 0; +} + +bool wxFFile::Error() const +{ + wxCHECK_MSG( IsOpened(), false, + wxT("wxFFile::Error(): file is closed!") ); + return ferror(m_fp) != 0; +} + #endif // wxUSE_FFILE diff --git a/tests/file/filefn.cpp b/tests/file/filefn.cpp index 659823806e..dec093a0eb 100644 --- a/tests/file/filefn.cpp +++ b/tests/file/filefn.cpp @@ -48,6 +48,8 @@ private: CPPUNIT_TEST( RenameFile ); CPPUNIT_TEST( ConcatenateFiles ); CPPUNIT_TEST( GetCwd ); + CPPUNIT_TEST( FileEof ); + CPPUNIT_TEST( FileError ); CPPUNIT_TEST_SUITE_END(); void GetTempFolder(); @@ -60,6 +62,8 @@ private: void RenameFile(); void ConcatenateFiles(); void GetCwd(); + void FileEof(); + void FileError(); // Helper methods void DoCreateFile(const wxString& filePath); @@ -423,6 +427,61 @@ void FileFunctionsTestCase::GetCwd() CPPUNIT_ASSERT( !cwd.IsEmpty() ); } +void FileFunctionsTestCase::FileEof() +{ + const wxString filename(wxT("horse.bmp")); + const wxString msg = wxString::Format(wxT("File: %s"), filename.c_str()); + const char *pUnitMsg = (const char*) msg.mb_str(wxConvUTF8); + wxFFile file(filename, wxT("r")); + // wxFFile::Eof must be false at start + CPPUNIT_ASSERT_MESSAGE( pUnitMsg, !file.Eof() ); + CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.SeekEnd() ); + // wxFFile::Eof returns true only after attempt to read last byte + char array[1]; + CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Read(array, 1) == 0 ); + CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Eof() ); + + CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Close() ); + // wxFFile::Eof after close should not cause crash but fail instead + bool failed = true; + try + { + file.Eof(); + failed = false; + } + catch (...) + { + } + CPPUNIT_ASSERT_MESSAGE( pUnitMsg, failed ); +} + +void FileFunctionsTestCase::FileError() +{ + const wxString filename(wxT("horse.bmp")); + const wxString msg = wxString::Format(wxT("File: %s"), filename.c_str()); + const char *pUnitMsg = (const char*) msg.mb_str(wxConvUTF8); + wxFFile file(filename, wxT("r")); + // wxFFile::Error must be false at start assuming file "horse.bmp" exists. + CPPUNIT_ASSERT_MESSAGE( pUnitMsg, !file.Error() ); + // Attempt to write to file opened in readonly mode should cause error + CPPUNIT_ASSERT_MESSAGE( pUnitMsg, !file.Write(filename) ); + CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Error() ); + + CPPUNIT_ASSERT_MESSAGE( pUnitMsg, file.Close() ); + // wxFFile::Error after close should not cause crash but fail instead + bool failed = true; + try + { + file.Error(); + failed = false; + } + catch (...) + { + } + CPPUNIT_ASSERT_MESSAGE( pUnitMsg, failed ); +} + + /* TODO: other file functions to test: