catch open issues earlier and add asserts

Author: samopolacek

cli/cppcheckexecutor.cpp

@@ -43,6 +43,7 @@
 #endif
 
 #include <algorithm>
+#include <cassert>
 #include <cstdio>
 #include <cstdlib> // EXIT_SUCCESS and EXIT_FAILURE
 #include <functional>
@@ -75,7 +76,20 @@ CppCheckExecutor::CppCheckExecutor()
 
 CppCheckExecutor::~CppCheckExecutor()
 {
-    delete mErrorOutput;
+    if (mErrorOutput) {
+        try
+        {
+            mErrorOutput->close();
+        }
+        catch (const std::ios_base::failure&)
+        {
+            assert(false);
+
+            // TODO report error
+        }
+
+        delete mErrorOutput;
+    }
 }
 
 bool CppCheckExecutor::parseFromArgs(Settings &settings, int argc, const char* const argv[])
@@ -266,8 +280,10 @@ int CppCheckExecutor::check_internal(CppCheck& cppcheck)
         mLatestProgressOutputTime = std::time(nullptr);
 
     if (!settings.outputFile.empty()) {
-        mErrorOutput = new std::ofstream(settings.outputFile);
+        mErrorOutput = new std::ofstream();
         mErrorOutput->exceptions(std::ios_base::failbit | std::ios_base::badbit);
+        mErrorOutput->open(settings.outputFile);
+        assert(mErrorOutput->is_open()); // If we ever disable exceptions, catch issues ASAP
     }
 
     if (settings.xml) {

gui/checkthread.cpp

@@ -76,8 +76,10 @@ static bool executeCommand(std::string exe, std::vector<std::string> args, std::
         output = process.readAllStandardOutput().toStdString();
 
     if (redirect.compare(0,3,"2> ") == 0) {
-        std::ofstream fout(redirect.substr(3));
+        std::ofstream fout();
         fout.exceptions(std::ios_base::failbit | std::ios_base::badbit);
+        fout.open(redirect.substr(3));
+        assert(fout.is_open()); // If we ever disable exceptions, catch issues ASAP
         fout << process.readAllStandardError().toStdString();
     }
     return process.exitCode() == 0;

lib/analyzerinfo.cpp

@@ -64,8 +64,11 @@ void AnalyzerInformation::writeFilesTxt(const std::string &buildDir, const std::
     std::map<std::string, unsigned int> fileCount;
 
     const std::string filesTxt(buildDir + "/files.txt");
-    std::ofstream fout(filesTxt);
+    std::ofstream fout;
     fout.exceptions(std::ios_base::failbit | std::ios_base::badbit);
+    fout.open(filesTxt);
+    assert(fout.is_open()); // If we ever disable exceptions, catch issues ASAP
+    
     for (const std::string &f : sourcefiles) {
         const std::string afile = getFilename(f);
         fout << afile << ".a" << (++fileCount[afile]) << "::" << Path::simplifyPath(Path::fromNativeSeparators(f)) << '\n';
@@ -154,9 +157,7 @@ bool AnalyzerInformation::analyzeFile(const std::string &buildDir, const std::st
         return false;
 
     mOutputStream.open(mAnalyzerInfoFile);
-    // If we ever disable exceptions on mOutputStream
-    // TODO: add this assert to all ofstreams
-    assert(mOutputStream.is_open());
+    assert(mOutputStream.is_open()); // If we ever disable exceptions on mOutputStream, catch issues ASAP
 
     mOutputStream << "<?xml version=\"1.0\"?>\n";
     mOutputStream << "<analyzerinfo hash=\"" << hash << "\">\n";

lib/cppcheck.cpp

@@ -276,14 +276,18 @@ static void createDumpFile(const Settings& settings,
     dumpFile = getDumpFileName(settings, filename);
 
     fdump.open(dumpFile);
+    assert(fdump.is_open()); // If we ever disable exceptions, catch issues ASAP
+
     // TODO: Technically, exceptions are allowed on the file so this is always false,
     // TODO: but the function is not aware of that as this function is not the owner of the file
     if (!fdump.is_open())
         return;
 
     {
-        std::ofstream fout(getCtuInfoFileName(dumpFile));
+        std::ofstream fout;
         fout.exceptions(std::ios_base::failbit | std::ios_base::badbit);
+        fout.open(getCtuInfoFileName(dumpFile));
+        assert(fout.is_open()); // If we ever disable exceptions, catch issues ASAP
     }
 
     std::string language;
@@ -399,7 +403,9 @@ CppCheck::CppCheck(ErrorLogger &errorLogger,
     , mTooManyConfigs(false)
     , mSimplify(true)
     , mExecuteCommand(std::move(executeCommand))
-{}
+{
+    mPlistFile.exceptions(std::ios_base::failbit | std::ios_base::badbit);
+}
 
 CppCheck::~CppCheck()
 {
@@ -411,7 +417,17 @@ CppCheck::~CppCheck()
 
     if (mPlistFile.is_open()) {
         mPlistFile << ErrorLogger::plistFooter();
-        mPlistFile.close();
+
+        try
+        {
+            mPlistFile.close();
+        }
+        catch (const std::ios_base::failure&)
+        {
+            assert(false);
+
+            // TODO report error
+        }
     }
 }
 
@@ -507,8 +523,10 @@ unsigned int CppCheck::check(const std::string &path)
         const std::string args2 = "-fsyntax-only -Xclang -ast-dump -fno-color-diagnostics " + flags + path;
         const std::string redirect2 = analyzerInfo.empty() ? std::string("2>&1") : ("2> " + clangStderr);
         if (!mSettings.buildDir.empty()) {
-            std::ofstream fout(clangcmd);
+            std::ofstream fout;
             fout.exceptions(std::ios_base::failbit | std::ios_base::badbit);
+            fout.open(clangcmd);
+            assert(fout.is_open()); // If we ever disable exceptions, catch issues ASAP
             fout << exe << " " << args2 << " " << redirect2 << std::endl;
         } else if (mSettings.verbose && !mSettings.quiet) {
             mErrorLogger.reportOut(exe + " " + args2);
@@ -539,8 +557,10 @@ unsigned int CppCheck::check(const std::string &path)
         }
 
         if (!mSettings.buildDir.empty()) {
-            std::ofstream fout(clangAst);
+            std::ofstream fout;
             fout.exceptions(std::ios_base::failbit | std::ios_base::badbit);
+            fout.open(clangAst);
+            assert(fout.is_open()); // If we ever disable exceptions, catch issues ASAP
             fout << output2 << std::endl;
         }
 
@@ -722,9 +742,12 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
                 filename2 = filename.substr(filename.rfind('/') + 1);
             else
                 filename2 = filename;
+            
             const std::size_t fileNameHash = std::hash<std::string> {}(filename);
             filename2 = mSettings.plistOutput + filename2.substr(0, filename2.find('.')) + "_" + std::to_string(fileNameHash) + ".plist";
             mPlistFile.open(filename2);
+            assert(mPlistFile.is_open());// If we ever disable exceptions, catch issues ASAP
+
             mPlistFile << ErrorLogger::plistHeader(version(), files);
         }
 
@@ -1429,8 +1452,11 @@ void CppCheck::executeAddons(const std::vector<std::string>& files)
 
     if (files.size() >= 2 || endsWith(files[0], ".ctu-info")) {
         fileList = Path::getPathFromFilename(files[0]) + FILELIST;
-        std::ofstream fout(fileList);
+        std::ofstream fout;
         fout.exceptions(std::ios_base::failbit | std::ios_base::badbit);
+        fout.open(fileList);
+        assert(fout.is_open()); // If we ever disable exceptions, catch issues ASAP
+
         for (const std::string& f: files)
             fout << f << std::endl;
     }
@@ -1689,8 +1715,12 @@ void CppCheck::analyseClangTidy(const ImportProject::FileSettings &fileSettings)
 
     if (!mSettings.buildDir.empty()) {
         const std::string analyzerInfoFile = AnalyzerInformation::getAnalyzerInfoFile(mSettings.buildDir, fileSettings.filename, emptyString);
-        std::ofstream fcmd(analyzerInfoFile + ".clang-tidy-cmd");
+
+        std::ofstream fcmd;
         fcmd.exceptions(std::ios_base::failbit | std::ios_base::badbit);
+        fcmd.open(analyzerInfoFile + ".clang-tidy-cmd");
+        assert(fcmd.is_open()); // If we ever disable exceptions, catch issues ASAP
+
         fcmd << istr.str();
     }
 

lib/summaries.cpp

@@ -86,8 +86,11 @@ std::string Summaries::create(const Tokenizer *tokenizer, const std::string &cfg
         const std::string::size_type pos = filename.rfind(".a");
         if (pos != std::string::npos) {
             filename[pos+1] = 's';
-            std::ofstream fout(filename);
+            std::ofstream fout;
             fout.exceptions(std::ios_base::failbit | std::ios_base::badbit);
+            fout.open(filename);
+            assert(fout.is_open()); // If we ever disable exceptions, catch issues ASAP
+
             fout << ostr.str();
         }
     }

test/helpers.cpp

@@ -54,8 +54,11 @@ ScopedFile::ScopedFile(std::string name, const std::string &content, std::string
 #endif
     }
 
-    std::ofstream of(mFullPath);
+    std::ofstream of;
     of.exceptions(std::ios_base::failbit | std::ios_base::badbit);
+    of.open(mFullPath);
+    assert(of.is_open()); // If we ever disable exceptions, catch issues ASAP
+
     of << content;
 }
 

tools/dmake.cpp

@@ -20,6 +20,7 @@
 
 #include <algorithm>
 #include <array>
+#include <cassert>
 #include <cstdlib>
 #include <fstream> // IWYU pragma: keep
 #include <functional>
@@ -214,9 +215,12 @@ static int write_vcxproj(const std::string &proj_name, const std::function<void(
             outstr.resize(pos+1, '\0');
     }
 
-    // treat as binary to prevent implicit line ending conversions
-    std::ofstream out(proj_name, std::ios::binary|std::ios::trunc);
+    std::ofstream out;
     out.exceptions(std::ios_base::failbit | std::ios_base::badbit);
+    // treat as binary to prevent implicit line ending conversions
+    out.open(proj_name, std::ios::binary|std::ios::trunc);
+    assert(out.is_open()); // If we ever disable exceptions, catch issues ASAP
+
     out << outstr;
 
     return EXIT_SUCCESS;
@@ -395,8 +399,10 @@ int main(int argc, char **argv)
 
     // QMAKE - lib/lib.pri
     {
-        std::ofstream fout1("lib/lib.pri");
+        std::ofstream fout1;
         fout1.exceptions(std::ios_base::failbit | std::ios_base::badbit);
+        fout1.open("lib/lib.pri");
+        assert(fout1.is_open()); // If we ever disable exceptions, catch issues ASAP
 
         fout1 << "# no manual edits - this file is autogenerated by dmake\n\n";
         fout1 << "include($$PWD/pcrerules.pri)\n";
@@ -418,8 +424,10 @@ int main(int argc, char **argv)
     }
 
     static const char makefile[] = "Makefile";
-    std::ofstream fout(makefile, std::ios_base::trunc);
+    std::ofstream fout;
     fout.exceptions(std::ios_base::failbit | std::ios_base::badbit);
+    fout.open(makefile, std::ios_base::trunc);
+    assert(fout.is_open()); // If we ever disable exceptions, catch issues ASAP
 
     fout << "# This file is generated by tools/dmake, do not edit.\n\n";