greatly improved Settings::loadCppcheckCfg() error handling
#5712
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,40 +40,87 @@ Settings::Settings() | |
| setCheckLevelNormal(); | ||
| } | ||
|
|
||
| std::string Settings::loadCppcheckCfg() | ||
| { | ||
| static const std::string cfgFilename = "cppcheck.cfg"; | ||
| std::string fileName; | ||
| #ifdef FILESDIR | ||
| if (Path::isFile(Path::join(FILESDIR, cfgFilename))) | ||
| fileName = Path::join(FILESDIR, cfgFilename); | ||
| #endif | ||
| // cppcheck-suppress knownConditionTrueFalse | ||
| if (fileName.empty()) { | ||
| fileName = Path::getPathFromFilename(exename) + cfgFilename; | ||
|
There was a problem hiding this comment.
There was a problem hiding this comment.
Oh right. That again. I ran into his in another context as well. Will take a look later today. There was a problem hiding this comment. If I may. I'm already working on it in a separate ticket and was just asking you to help me clarify your thought process behind this particular change. There was a problem hiding this comment. I see. I would have just hot fixed this this for now. As it would have also tied into my upcoming changes to fix https://trac.cppcheck.net/ticket/12240 and #4424 I would have done any more refactoring within that. |
||
| if (!Path::isFile(fileName)) | ||
| return ""; | ||
| } | ||
|
|
||
| std::ifstream fin(fileName); | ||
| if (!fin.is_open()) | ||
| return "could not open file"; | ||
| picojson::value json; | ||
| fin >> json; | ||
| { | ||
| const std::string& lastErr = picojson::get_last_error(); | ||
| if (!lastErr.empty()) | ||
| return "not a valid JSON - " + lastErr; | ||
| } | ||
| const picojson::object& obj = json.get<picojson::object>(); | ||
| { | ||
| const picojson::object::const_iterator it = obj.find("productName"); | ||
| if (it != obj.cend()) { | ||
| const auto& v = it->second; | ||
| if (!v.is<std::string>()) | ||
| return "'productName' is not a string"; | ||
| cppcheckCfgProductName = v.get<std::string>(); | ||
| } | ||
| } | ||
| { | ||
| const picojson::object::const_iterator it = obj.find("about"); | ||
| if (it != obj.cend()) { | ||
| const auto& v = it->second; | ||
| if (!v.is<std::string>()) | ||
| return "'about' is not a string"; | ||
| cppcheckCfgAbout = v.get<std::string>(); | ||
| } | ||
| } | ||
| { | ||
| const picojson::object::const_iterator it = obj.find("addons"); | ||
| if (it != obj.cend()) { | ||
| const auto& entry = it->second; | ||
| if (!entry.is<picojson::array>()) | ||
| return "'addons' is not an array"; | ||
| for (const picojson::value &v : entry.get<picojson::array>()) | ||
| { | ||
| if (!v.is<std::string>()) | ||
| return "'addons' array entry is not a string"; | ||
| const std::string &s = v.get<std::string>(); | ||
| if (!Path::isAbsolute(s)) | ||
| addons.emplace(Path::join(Path::getPathFromFilename(fileName), s)); | ||
| else | ||
| addons.emplace(s); | ||
| } | ||
| } | ||
| } | ||
| { | ||
| const picojson::object::const_iterator it = obj.find("suppressions"); | ||
| if (it != obj.cend()) { | ||
| const auto& entry = it->second; | ||
| if (!entry.is<picojson::array>()) | ||
| return "'suppressions' is not an array"; | ||
| for (const picojson::value &v : entry.get<picojson::array>()) | ||
| { | ||
| if (!v.is<std::string>()) | ||
| return "'suppressions' array entry is not a string"; | ||
| const std::string &s = v.get<std::string>(); | ||
| const std::string err = nomsg.addSuppressionLine(s); | ||
| if (!err.empty()) | ||
| return "could not parse suppression '" + s + "' - " + err; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return ""; | ||
| } | ||
|
|
||
| std::string Settings::parseEnabled(const std::string &str, std::tuple<SimpleEnableGroup<Severity>, SimpleEnableGroup<Checks>> &groups) | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@firewave May I ask you why do we need to create the local settings here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we do not accidentally load the config multiple times times. That should be done in a deterministic way once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay agree on that one. But you only creating local variable so it's not working.
adding
staticto it will help though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is not working? We only need to compare s single string from the object. Also this is only done during the parsing of the command-line options.
What? The method or the object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both but as you've already marked the method you need to add it only here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the object it would not help as it still requires the function call.
I think I did look at the method but I am not sure. It is also just a limited usage.