From 9f977ed41971ab9c54f33211efe4ec1e2bf19289 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Thu, 24 Oct 2019 21:48:45 -0400 Subject: [PATCH] Codacy/Cppcheck cleanups and badge. --- .codacy.yml | 3 +++ MANIFEST.SKIP | 1 + README.pod | 3 +++ ci/build_verilator.sh | 18 +++++++++--------- include/verilatedos.h | 2 +- src/V3Changed.cpp | 1 + src/V3EmitC.cpp | 2 +- src/V3EmitXml.cpp | 2 +- src/V3GraphDfa.cpp | 4 ++++ src/V3Life.cpp | 3 +-- src/V3LinkDot.cpp | 2 +- src/V3Param.cpp | 2 +- src/V3ParseImp.cpp | 2 +- src/V3TSP.cpp | 1 - src/V3Trace.cpp | 1 - src/V3Unknown.cpp | 1 - test_regress/t/t_clk_2in.cpp | 2 +- test_regress/t/t_dpi_open_c.cpp | 12 ++++++------ test_regress/t/t_dpi_var.cpp | 2 +- test_regress/t/t_protect_ids_c.cpp | 1 - test_regress/t/t_scope_map.cpp | 3 ++- 21 files changed, 38 insertions(+), 30 deletions(-) create mode 100644 .codacy.yml diff --git a/.codacy.yml b/.codacy.yml new file mode 100644 index 000000000..b40076535 --- /dev/null +++ b/.codacy.yml @@ -0,0 +1,3 @@ +--- +exclude_paths: + - 'include/vltstd/**' diff --git a/MANIFEST.SKIP b/MANIFEST.SKIP index e14400721..e6f73b20a 100644 --- a/MANIFEST.SKIP +++ b/MANIFEST.SKIP @@ -14,6 +14,7 @@ .*\.key .*\.vcd .*\.1 +\.codacy\.yml \.travis\.yml /build/ /obj_dir/ diff --git a/README.pod b/README.pod index a0b31384b..011e1e70d 100644 --- a/README.pod +++ b/README.pod @@ -3,6 +3,9 @@ =pod +=for html +=for html +=for html =for html =head1 NAME diff --git a/ci/build_verilator.sh b/ci/build_verilator.sh index ffdfd90b5..338e5e503 100755 --- a/ci/build_verilator.sh +++ b/ci/build_verilator.sh @@ -16,23 +16,23 @@ # not be used as the script relies on Git revisions for caching. set -e -if [ -z ${VERILATOR_NUM_JOBS} ]; then +if [ -z "${VERILATOR_NUM_JOBS}" ]; then VERILATOR_NUM_JOBS=$(nproc) fi # Caching would be simpler if we installed without VERILATOR_ROOT, but # it's needed for driver.pl outside of the repo -if [ -z ${VERILATOR_ROOT} ]; then +if [ -z "${VERILATOR_ROOT}" ]; then echo "VERILATOR_ROOT not set" exit -1 fi -if [ -z ${VERILATOR_CACHE} ]; then +if [ -z "${VERILATOR_CACHE}" ]; then echo "VERILATOR_CACHE not set" exit -1 fi -VERILATOR_REV=$(cd ${VERILATOR_ROOT} && git rev-parse HEAD) +VERILATOR_REV=$(cd "${VERILATOR_ROOT}" && git rev-parse HEAD) echo "Found Verilator rev ${VERILATOR_REV}" CACHED_REV_FILE=${VERILATOR_CACHE}/.rev.txt @@ -50,15 +50,15 @@ if [[ ! -f ${CACHED_REV_FILE} || \ cd ${VERILATOR_ROOT} autoconf && ./configure ${VERILATOR_CONFIG_FLAGS} && make -j ${VERILATOR_NUM_JOBS} # Copy the Verilator build artifacts - mkdir -p ${VERILATOR_CACHE} - rm -rf ${VERILATOR_CACHE}/* - cp bin/*bin* ${VERILATOR_CACHE} + mkdir -p "${VERILATOR_CACHE}" + rm -rf "${VERILATOR_CACHE}/*" + cp bin/*bin* "${VERILATOR_CACHE}" # Remember the Git revision echo ${VERILATOR_REV} > ${CACHED_REV_FILE} else echo "Using cached Verilator" - cd ${VERILATOR_ROOT} + cd "${VERILATOR_ROOT}" # Create include/verilated_config.h and maybe other things autoconf && ./configure ${VERILATOR_CONFIG_FLAGS} - cp ${VERILATOR_CACHE}/* bin + cp "${VERILATOR_CACHE}/*" bin fi diff --git a/include/verilatedos.h b/include/verilatedos.h index 5aeac7ee0..a7e918779 100644 --- a/include/verilatedos.h +++ b/include/verilatedos.h @@ -162,7 +162,7 @@ //========================================================================= // C++-2011 -#if __cplusplus >= 201103L || defined(__GXX_EXPERIMENTAL_CXX0X__) +#if __cplusplus >= 201103L || defined(__GXX_EXPERIMENTAL_CXX0X__) || defined(VL_CPPCHECK) # define VL_EQ_DELETE = delete # define vl_unique_ptr std::unique_ptr // By default we use std:: types in C++11. diff --git a/src/V3Changed.cpp b/src/V3Changed.cpp index 81d781624..566f2fd22 100644 --- a/src/V3Changed.cpp +++ b/src/V3Changed.cpp @@ -222,6 +222,7 @@ public: m_newRvEqnp->deleteTree(); } virtual ~ChangedInsertVisitor() {} + VL_UNCOPYABLE(ChangedInsertVisitor); }; //###################################################################### diff --git a/src/V3EmitC.cpp b/src/V3EmitC.cpp index f90ba3c59..fb4e9bd58 100644 --- a/src/V3EmitC.cpp +++ b/src/V3EmitC.cpp @@ -1622,7 +1622,7 @@ void EmitCStmts::displayArg(AstNode* dispp, AstNode** elistp, bool isScan, if (argp->widthMin() > VL_VALUE_STRING_MAX_WIDTH) { dispp->v3error("Exceeded limit of "+cvtToStr(VL_VALUE_STRING_MAX_WIDTH)+" bits for any $display-like arguments"); } - if (argp && argp->widthMin()>8 && fmtLetter=='c') { + if (argp->widthMin() > 8 && fmtLetter == 'c') { // Technically legal, but surely not what the user intended. argp->v3warn(WIDTH, dispp->verilogKwd()<<"of %c format of > 8 bit value"); } diff --git a/src/V3EmitXml.cpp b/src/V3EmitXml.cpp index 52b12298a..298c93e1f 100644 --- a/src/V3EmitXml.cpp +++ b/src/V3EmitXml.cpp @@ -253,7 +253,7 @@ public: // Xml output m_os<<"\n"; for (std::deque::iterator it = m_nodeModules.begin(); - it != m_nodeModules.end(); it++) { + it != m_nodeModules.end(); ++it) { m_os<<"filenameLetters() <<"\" filename=\""<<(*it)->filename() <<"\" language=\""<<(*it)->language().ascii()<<"\"/>\n"; diff --git a/src/V3GraphDfa.cpp b/src/V3GraphDfa.cpp index 8a7423fb2..f3607a496 100644 --- a/src/V3GraphDfa.cpp +++ b/src/V3GraphDfa.cpp @@ -384,7 +384,11 @@ void DfaGraph::nfaToDfa() { class DfaGraphReduce : GraphAlg<> { private: // METHODS +#ifdef VL_CPPCHECK + static int debug() { return 9; } +#else static int debug() { return 0; } +#endif DfaGraph* graphp() { return static_cast(m_graphp); } bool isDead(DfaVertex* vertexp) { diff --git a/src/V3Life.cpp b/src/V3Life.cpp index 580d391e6..9d2518b14 100644 --- a/src/V3Life.cpp +++ b/src/V3Life.cpp @@ -346,13 +346,12 @@ private: { m_lifep = ifLifep; iterateAndNextNull(nodep->ifsp()); - m_lifep = prevLifep; } { m_lifep = elseLifep; iterateAndNextNull(nodep->elsesp()); - m_lifep = prevLifep; } + m_lifep = prevLifep; UINFO(4," join "<dualBranch(ifLifep, elseLifep); diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index f1c3a120a..9e04de51f 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -1121,7 +1121,7 @@ class LinkDotFindVisitor : public AstNVisitor { ins = true; } else if (findvarp != nodep) { UINFO(4,"DupVar: "<parentp() == m_curSymp // Only when on same level + if (foundp->parentp() == m_curSymp // Only when on same level && !foundp->imported()) { // and not from package nodep->v3error("Duplicate declaration of enum value: "<prettyName()<warnContextPrimary()<prettyNameQ()); } else { UINFO(9,"Parameter type assignment expr="<sameTree(origp)) { + if (exprp->sameTree(origp)) { // Setting parameter to its default value. Just ignore it. // This prevents making additional modules, and makes coverage more // obvious as it won't show up under a unique module page name. diff --git a/src/V3ParseImp.cpp b/src/V3ParseImp.cpp index 0c070e975..89b7ec095 100644 --- a/src/V3ParseImp.cpp +++ b/src/V3ParseImp.cpp @@ -191,7 +191,7 @@ size_t V3ParseImp::ppInputToLex(char* buf, size_t max_size) { m_ppBuffers.push_front(remainder); // Put back remainder for next time len = (max_size-got); } - strncpy(buf+got, front.c_str(), len); + memcpy(buf+got, front.c_str(), len); got += len; } if (debug()>=9) { diff --git a/src/V3TSP.cpp b/src/V3TSP.cpp index d08fe7e3b..7b94fc840 100644 --- a/src/V3TSP.cpp +++ b/src/V3TSP.cpp @@ -505,7 +505,6 @@ void V3TSP::tspSort(const V3TSP::StateVec& states, V3TSP::StateVec* resultp) { if (max_cost_idx == resultp->size() - 1) { // List is already rotated for minimum cost. stop. - UASSERT(resultp->size() == resultp->size(), "sizes should match"); return; } diff --git a/src/V3Trace.cpp b/src/V3Trace.cpp index ccb8a72c1..b6d10aaeb 100644 --- a/src/V3Trace.cpp +++ b/src/V3Trace.cpp @@ -756,7 +756,6 @@ public: m_chgSubStmts = 0; m_activityNumber = 0; m_code = 0; - m_finding = false; m_funcNum = 0; iterate(nodep); } diff --git a/src/V3Unknown.cpp b/src/V3Unknown.cpp index b8523e9ec..0cd82437f 100644 --- a/src/V3Unknown.cpp +++ b/src/V3Unknown.cpp @@ -435,7 +435,6 @@ private: // ARRAYSEL(...) -> ARRAYSEL(COND(LT(bitbitp()->unlinkFrBack(&replaceHandle); - V3Number zeronum (nodep, bitp->width(), 0); AstNode* newp = new AstCondBound(bitp->fileline(), condp, bitp, new AstConst(bitp->fileline(), diff --git a/test_regress/t/t_clk_2in.cpp b/test_regress/t/t_clk_2in.cpp index 93f69a74d..182de5b45 100644 --- a/test_regress/t/t_clk_2in.cpp +++ b/test_regress/t/t_clk_2in.cpp @@ -23,7 +23,7 @@ void clockit(int clk1, int clk0) { topp->c0 = clk0; #endif #ifdef TEST_VERBOSE - printf("[%d] c1=%d c0=%d\n", main_time, clk1, clk0); + printf("[%d] c1=%u c0=%u\n", main_time, clk1, clk0); #endif topp->eval(); main_time++; diff --git a/test_regress/t/t_dpi_open_c.cpp b/test_regress/t/t_dpi_open_c.cpp index ad57dd63f..ca2a07c5c 100644 --- a/test_regress/t/t_dpi_open_c.cpp +++ b/test_regress/t/t_dpi_open_c.cpp @@ -176,16 +176,16 @@ void _dpii_all(int c, int p, int u, const svOpenArrayHandle i, const svOpenArray vec[0].a = (~vec[0].a); vec[1].a = (~vec[1].a); vec[2].a = (~vec[2].a) & 0x7fffffff; - vec[0].b = vec[0].b; - vec[1].b = vec[1].b; - vec[2].b = vec[2].b; + //vec[0].b = vec[0].b; + //vec[1].b = vec[1].b; + //vec[2].b = vec[2].b; #else vec[0].aval = (~vec[0].aval); vec[1].aval = (~vec[1].aval); vec[2].aval = (~vec[2].aval) & 0x7fffffff; - vec[0].bval = vec[0].bval; - vec[1].bval = vec[1].bval; - vec[2].bval = vec[2].bval; + //vec[0].bval = vec[0].bval; + //vec[1].bval = vec[1].bval; + //vec[2].bval = vec[2].bval; #endif svPutLogicArrElemVecVal(o, vec, a, b, c); } diff --git a/test_regress/t/t_dpi_var.cpp b/test_regress/t/t_dpi_var.cpp index 056f8f825..c7993be0e 100644 --- a/test_regress/t/t_dpi_var.cpp +++ b/test_regress/t/t_dpi_var.cpp @@ -86,8 +86,8 @@ void mon_register_b(const char* namep, int isOut) { extern "C" void mon_register_done(); void mon_register_done() { - const char* modp = svGetNameFromScope(svGetScope()); #ifdef TEST_VERBOSE + const char* modp = svGetNameFromScope(svGetScope()); VL_PRINTF("- mon_register_done('%s');\n", modp); #endif // Print list of all signals - if we didn't register2 anything we'd pick them off here diff --git a/test_regress/t/t_protect_ids_c.cpp b/test_regress/t/t_protect_ids_c.cpp index 00657d5bf..63484f67f 100644 --- a/test_regress/t/t_protect_ids_c.cpp +++ b/test_regress/t/t_protect_ids_c.cpp @@ -45,7 +45,6 @@ int dpii_a_func(int i) { } int dpii_a_task(int i, int* op) { - int o = 0; (void)dpix_a_task(i, op); return 0; } diff --git a/test_regress/t/t_scope_map.cpp b/test_regress/t/t_scope_map.cpp index 53de0e2a6..6047e3cb9 100644 --- a/test_regress/t/t_scope_map.cpp +++ b/test_regress/t/t_scope_map.cpp @@ -41,7 +41,8 @@ int main(int argc, char **argv, char **env) { ++main_time; const VerilatedScopeNameMap* scopeMapp = Verilated::scopeNameMap(); - for (VerilatedScopeNameMap::const_iterator it = scopeMapp->begin(); it != scopeMapp->end(); it++) { + for (VerilatedScopeNameMap::const_iterator it = scopeMapp->begin(); + it != scopeMapp->end(); ++it) { #ifdef TEST_VERBOSE VL_PRINTF("---------------------------------------------\n"); VL_PRINTF("Scope = %s\n", it->first);