From 72e1acac7ac4458487121019215c7e3806bc5f53 Mon Sep 17 00:00:00 2001 From: Kamil Dudka Date: Thu, 12 Dec 2024 09:09:19 +0100 Subject: [PATCH 1/3] instream: pass fileName by reference ... to eliminate unnecessary string copies Suggested by Coverity: ``` Error: COPY_INSTEAD_OF_MOVE: src/cslinker.cc:236:29: copy_constructor_call: "fnCwe" is passed-by-value as parameter to "std::__cxx11::basic_string, std::allocator >::basic_string(std::__cxx11::basic_string, std::allocator > const &)", when it could be moved instead. src/cslinker.cc:236:29: remediation: Use "std::move(""fnCwe"")" instead of "fnCwe". Error: COPY_INSTEAD_OF_MOVE: src/cslinker.cc:249:29: copy_constructor_call: "fnImp" is passed-by-value as parameter to "std::__cxx11::basic_string, std::allocator >::basic_string(std::__cxx11::basic_string, std::allocator > const &)", when it could be moved instead. src/cslinker.cc:249:29: remediation: Use "std::move(""fnImp"")" instead of "fnImp". ``` Related: https://github.com/csutils/csdiff/pull/216 --- src/lib/instream.cc | 4 ++-- src/lib/instream.hh | 2 +- src/lib/parser-json-cov.cc | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/lib/instream.cc b/src/lib/instream.cc index 3380e649..ab9225f5 100644 --- a/src/lib/instream.cc +++ b/src/lib/instream.cc @@ -19,8 +19,8 @@ #include "instream.hh" -InStream::InStream(std::string fileName, const bool silent): - fileName_(std::move(fileName)), +InStream::InStream(const std::string &fileName, const bool silent): + fileName_(fileName), silent_(silent), str_((fileName_ == "-") ? std::cin diff --git a/src/lib/instream.hh b/src/lib/instream.hh index a842bcee..fcbb480b 100644 --- a/src/lib/instream.hh +++ b/src/lib/instream.hh @@ -38,7 +38,7 @@ struct InFileException { class InStream { public: - InStream(std::string fileName, bool silent = false); + InStream(const std::string &fileName, bool silent = false); InStream(std::istringstream &str, bool silent = false); ~InStream() = default; diff --git a/src/lib/parser-json-cov.cc b/src/lib/parser-json-cov.cc index 8fdb9905..99bee8d9 100644 --- a/src/lib/parser-json-cov.cc +++ b/src/lib/parser-json-cov.cc @@ -89,4 +89,3 @@ bool CovTreeDecoder::readNode(Defect *def) return true; } - From ba468e9edcfe5677a845986185a76ca1f134abb7 Mon Sep 17 00:00:00 2001 From: Kamil Dudka Date: Thu, 12 Dec 2024 09:12:36 +0100 Subject: [PATCH 2/3] lib: use std::move() where appropriate Suggested by Coverity: ``` Error: COPY_INSTEAD_OF_MOVE: src/lib/filter.cc:114:25: copy_constructor_call: "evt" is copied and then passed-by-reference as parameter to STL insertion function "std::vector >::push_back(std::vector >::value_type const &)", when it could be moved instead. src/lib/filter.cc:114:25: remediation: Use "std::move(""evt"")" instead of "evt". Error: COPY_INSTEAD_OF_MOVE: src/lib/filter.cc:205:29: copy_constructor_call: "evt" is copied and then passed-by-reference as parameter to STL insertion function "std::set, std::allocator >::insert(std::set, std::allocator >::value_type const &)", when it could be moved instead. src/lib/filter.cc:205:29: remediation: Use "std::move(""evt"")" instead of "evt". Error: COPY_INSTEAD_OF_MOVE: src/lib/parser-cov.cc:428:28: copy_constructor_call: "str" is copied and then passed-by-reference as parameter to STL insertion function "std::set, std::allocator >, std::less, std::allocator > >, std::allocator, std::allocator > > >::insert(std::set, std::allocator >, std::less, std::allocator > >, std::allocator, std::allocator > > >::value_type const &)", when it could be moved instead. src/lib/parser-cov.cc:428:28: remediation: Use "std::move(""str"")" instead of "str". Error: COPY_INSTEAD_OF_MOVE: src/lib/parser-gcc.cc:552:46: copy_constructor_call: "evt" is copied and then passed-by-reference as parameter to STL insertion function "std::vector >::push_back(std::vector >::value_type const &)", when it could be moved instead. src/lib/parser-gcc.cc:552:46: remediation: Use "std::move(""evt"")" instead of "evt". Error: COPY_INSTEAD_OF_MOVE: src/lib/parser-gcc.cc:558:46: copy_constructor_call: "evt" is copied and then passed-by-reference as parameter to STL insertion function "std::vector >::push_back(std::vector >::value_type const &)", when it could be moved instead. src/lib/parser-gcc.cc:558:46: remediation: Use "std::move(""evt"")" instead of "evt". Error: COPY_INSTEAD_OF_MOVE: src/lib/parser-json-sarif.cc:205:13: copy_assignment_call: "uri" is copied in call to copy assignment for class "std::string const", when it could be moved instead. src/lib/parser-json-sarif.cc:205:30: remediation: Use "std::move(""uri"")" instead of "uri". Error: COPY_INSTEAD_OF_MOVE: src/lib/parser-json-sarif.cc:250:32: copy_constructor_call: "evt" is copied and then passed-by-reference as parameter to STL insertion function "std::vector >::push_back(std::vector >::value_type const &)", when it could be moved instead. src/lib/parser-json-sarif.cc:250:32: remediation: Use "std::move(""evt"")" instead of "evt". Error: COPY_INSTEAD_OF_MOVE: src/lib/parser-json-simple.cc:157:30: copy_constructor_call: "evt" is copied and then passed-by-reference as parameter to STL insertion function "std::vector >::push_back(std::vector >::value_type const &)", when it could be moved instead. src/lib/parser-json-simple.cc:157:30: remediation: Use "std::move(""evt"")" instead of "evt". Error: COPY_INSTEAD_OF_MOVE: src/lib/parser-json-zap.cc:144:9: copy_assignment_call: "version" is copied in call to copy assignment for class "std::string const", when it could be moved instead. src/lib/parser-json-zap.cc:144:49: remediation: Use "std::move(""version"")" instead of "version". Error: COPY_INSTEAD_OF_MOVE: src/lib/parser-xml-valgrind.cc:61:9: copy_assignment_call: "argVal" is copied in call to copy assignment for class "std::string const", when it could be moved instead. src/lib/parser-xml-valgrind.cc:61:17: remediation: Use "std::move(""argVal"")" instead of "argVal". Error: COPY_INSTEAD_OF_MOVE: src/lib/parser-xml-valgrind.cc:227:32: copy_constructor_call: "noteEvt" is copied and then passed-by-reference as parameter to STL insertion function "std::vector >::push_back(std::vector >::value_type const &)", when it could be moved instead. src/lib/parser-xml-valgrind.cc:227:32: remediation: Use "std::move(""noteEvt"")" instead of "noteEvt". ``` Related: https://github.com/csutils/csdiff/pull/216 --- src/lib/filter.cc | 4 ++-- src/lib/parser-cov.cc | 2 +- src/lib/parser-gcc.cc | 4 ++-- src/lib/parser-json-sarif.cc | 4 ++-- src/lib/parser-json-simple.cc | 2 +- src/lib/parser-json-zap.cc | 2 +- src/lib/parser-xml-valgrind.cc | 4 ++-- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/lib/filter.cc b/src/lib/filter.cc index 8659772f..f9f06cb8 100644 --- a/src/lib/filter.cc +++ b/src/lib/filter.cc @@ -111,7 +111,7 @@ void appendCtxLines( evt.event = "#"; evt.msg = str.str(); evt.verbosityLevel = /* not a key event */ 1; - pDst->push_back(evt); + pDst->push_back(std::move(evt)); } } @@ -202,7 +202,7 @@ bool DuplicateFilter::matchDef(const Defect &def) evt.fileName = MsgFilter::inst().filterPath(evt.fileName); evt.msg = MsgFilter::inst().filterMsg(evt.msg, def.checker); - return d->lookup.insert(evt)./* inserted */second; + return d->lookup.insert(std::move(evt))./* inserted */second; } diff --git a/src/lib/parser-cov.cc b/src/lib/parser-cov.cc index a6f97d72..5d4e4dc8 100644 --- a/src/lib/parser-cov.cc +++ b/src/lib/parser-cov.cc @@ -425,7 +425,7 @@ bool KeyEventDigger::guessKeyEvent(Defect *def) // no override for the checker -> match the lowered checker name std::string str(def->checker); boost::algorithm::to_lower(str); - defKeyEvent.insert(str); + defKeyEvent.insert(std::move(str)); } // look for an explicitly defined key event diff --git a/src/lib/parser-gcc.cc b/src/lib/parser-gcc.cc index 16094f26..2f94d465 100644 --- a/src/lib/parser-gcc.cc +++ b/src/lib/parser-gcc.cc @@ -549,13 +549,13 @@ bool BasicGccParser::getNext(Defect *pDef) case T_INC: case T_SCOPE: done = this->exportAndReset(pDef); - defCurrent_.events.push_back(evt); + defCurrent_.events.push_back(std::move(evt)); break; case T_MSG: done = this->exportAndReset(pDef); defCurrent_.keyEventIdx = defCurrent_.events.size(); - defCurrent_.events.push_back(evt); + defCurrent_.events.push_back(std::move(evt)); hasKeyEvent_ = true; break; diff --git a/src/lib/parser-json-sarif.cc b/src/lib/parser-json-sarif.cc index 33676aea..0e71e721 100644 --- a/src/lib/parser-json-sarif.cc +++ b/src/lib/parser-json-sarif.cc @@ -202,7 +202,7 @@ static void sarifReadLocation(DefEvent *pEvt, const pt::ptree &loc) const auto uri = valueOf(*al, "uri"); if (!uri.empty()) // read file name - pEvt->fileName = uri; + pEvt->fileName = std::move(uri); } const pt::ptree *reg; @@ -247,7 +247,7 @@ static void sarifReadComments(Defect *pDef, const pt::ptree &relatedLocs) continue; evt.verbosityLevel = 1; - pDef->events.push_back(evt); + pDef->events.push_back(std::move(evt)); } } diff --git a/src/lib/parser-json-simple.cc b/src/lib/parser-json-simple.cc index 097329b7..9b973670 100644 --- a/src/lib/parser-json-simple.cc +++ b/src/lib/parser-json-simple.cc @@ -154,7 +154,7 @@ bool SimpleTreeDecoder::readNode(Defect *def) if (-1 == evt.verbosityLevel) verbosityLevelNeedsInit = true; - evtListDst.push_back(evt); + evtListDst.push_back(std::move(evt)); } // read "defect_id", "cwe", and "function" if available diff --git a/src/lib/parser-json-zap.cc b/src/lib/parser-json-zap.cc index 89f3cca0..dac9669b 100644 --- a/src/lib/parser-json-zap.cc +++ b/src/lib/parser-json-zap.cc @@ -141,7 +141,7 @@ void ZapTreeDecoder::readScanProps( { const auto version = valueOf(*root, "@version"); if (!version.empty()) - (*pDst)["analyzer-version-owasp-zap"] = version; + (*pDst)["analyzer-version-owasp-zap"] = std::move(version); d->timeStamp = valueOf(*root, "@generated"); } diff --git a/src/lib/parser-xml-valgrind.cc b/src/lib/parser-xml-valgrind.cc index 54d6c26b..b9d622d7 100644 --- a/src/lib/parser-xml-valgrind.cc +++ b/src/lib/parser-xml-valgrind.cc @@ -58,7 +58,7 @@ bool /* continue */ skipLdArgs( goto skip_arg; // record path of the real binary being executed - *pExe = argVal; + *pExe = std::move(argVal); ++(*pIt); return /* continue */ (itEnd != *pIt); @@ -224,7 +224,7 @@ void readStack(Defect *pDef, const pt::ptree &stackNode) } // finally push the "note" event - pDef->events.push_back(noteEvt); + pDef->events.push_back(std::move(noteEvt)); } } From a61fae381169b4d0ca016e4fa1905c22059ec440 Mon Sep 17 00:00:00 2001 From: Kamil Dudka Date: Thu, 12 Dec 2024 09:16:06 +0100 Subject: [PATCH 3/3] cwe-mapper: suppres a false positive of Coverity ``` Error: PASS_BY_VALUE (CWE-398): src/lib/cwe-mapper.cc:38:45: pass_by_value: Passing parameter def of type "Defect" (size 200 bytes) by value, which exceeds the low threshold of 128 bytes. ``` Closes: https://github.com/csutils/csdiff/pull/216 --- src/lib/cwe-mapper.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/cwe-mapper.cc b/src/lib/cwe-mapper.cc index 32e7d097..f80dff75 100644 --- a/src/lib/cwe-mapper.cc +++ b/src/lib/cwe-mapper.cc @@ -35,6 +35,7 @@ struct CweMap::Private { bool detectedByTool(Defect def, const char *tool) const; }; +// coverity[pass_by_value] bool CweMap::Private::detectedByTool(Defect def, const char *tool) const { // detect tool in case it is not explicitly specified