csmith meets cppcheck, the static analyser

[dcb314]

cppcheck is a static analyser for C and C++ code.

Here is an example of some of its output:

trunk/src/StringUtils.cpp:115:40: performance: Function parameter 'str' should be passed by const reference. [passedByValue]
trunk/src/StringUtils.cpp:134:40: performance: Function parameter 'str' should be passed by const reference. [passedByValue]
trunk/src/StringUtils.cpp:153:44: performance: Function parameter 'str' should be passed by const reference. [passedByValue]
trunk/src/StringUtils.h:64:40: performance: Function parameter 'str' should be passed by const reference. [passedByValue]
trunk/src/StringUtils.h:66:40: performance: Function parameter 'str' should be passed by const reference. [passedByValue]
trunk/src/StringUtils.h:68:44: performance: Function parameter 'str' should be passed by const reference. [passedByValue]

And here is a diff for those performance problems:

diff --git a/src/StringUtils.cpp b/src/StringUtils.cpp
index 201c493..8ef06f7 100644
--- a/src/StringUtils.cpp
+++ b/src/StringUtils.cpp
@@ -112,7 +112,7 @@ StringUtils::find_any_char(const string &s, size_t pos, const string& to_match)
}

void
-StringUtils::split_string(const string str, vector &v, const char sep_char)
+StringUtils::split_string(const string & str, vector &v, const char sep_char)
{
size_t pos = 0;
size_t start_pos = 0;
@@ -131,7 +131,7 @@ StringUtils::split_string(const string str, vector &v, const char sep_ch
}

void
-StringUtils::split_string(const string str, vector &v, string sep_chars)
+StringUtils::split_string(const string & str, vector &v, string sep_chars)
{
size_t pos = 0;
size_t start_pos = 0;
@@ -150,7 +150,7 @@ StringUtils::split_string(const string str, vector &v, string sep_chars)
}

void
-StringUtils::split_int_string(const string str, vector &values, string sep_chars)
+StringUtils::split_int_string(const string & str, vector &values, string sep_chars)
{
size_t pos = 0;
size_t start_pos = 0;

diff --git a/src/StringUtils.h b/src/StringUtils.h
index 66563a7..763d25c 100644
--- a/src/StringUtils.h
+++ b/src/StringUtils.h
@@ -61,11 +61,11 @@ public:

    static size_t find_any_char(const string &s, size_t pos, const string& to_match);

•   static void split_string(const string str, vector<string> &v, const char sep_char);
  

•   static void split_string(const string & str, vector<string> &v, const char sep_char);
  

•   static void split_string(const string str, vector<string> &v, string sep_chars);
  

•   static void split_string(const string & str, vector<string> &v, string sep_chars);
  

•   static void split_int_string(const string str, vector<int> &values, string sep_chars);
  

•   static void split_int_string(const string & str, vector<int> &values, string sep_chars);
  
    static void chop(string& str);
  

[dcb314]

Perhaps more interestingly, a possible bug:

trunk/src/Expression.cpp:319:13: warning: Member variable 'Expression::expr_id' is not initialized in the copy constructor. [uninitMemberVar]

Source code is

Expression::Expression(eTermType e) :
term_type(e),
expr_id(eid++),
cast_type(NULL)
{
// Nothing to do.
}

Expression::Expression(const Expression &expr) :
term_type(expr.term_type),
cast_type(NULL)
{

}

[eeide]

This does look like a bug! Thank you for reporting it!

[dcb314]

cppcheck also found some unused variables. Here is a git diff

diff --git a/src/DefaultOutputMgr.cpp b/src/DefaultOutputMgr.cpp
index a1c6403..0c104fb 100644
--- a/src/DefaultOutputMgr.cpp
+++ b/src/DefaultOutputMgr.cpp
@@ -149,7 +149,7 @@ DefaultOutputMgr::OutputAllHeaders()
vector<ofstream *>::iterator j;
for (j = outs.begin(); j != outs.end(); ++j) {
ofstream *out = (*j);

•   string prefix = "extern ";
  

•   // string prefix = "extern ";
    // OutputGlobalVariablesDecls(*out, prefix);
    OutputForwardDeclarations(*out);
    *out << std::endl;
  

diff --git a/src/FunctionInvocation.cpp b/src/FunctionInvocation.cpp
index aa99588..cb628d8 100644
--- a/src/FunctionInvocation.cpp
+++ b/src/FunctionInvocation.cpp
@@ -536,7 +536,7 @@ FunctionInvocation::visit_facts(vector<const Fact*>& inputs, CGContext& cg_conte
}
if (ok && is_func_call) {
// make a copy of env

•   vector<const Fact*> inputs_copy = inputs;
  

•   // vector<const Fact*> inputs_copy = inputs;
    const FunctionInvocationUser* func_call = dynamic_cast<const FunctionInvocationUser*>(this);
    Effect effect_accum;
    //CGContext new_context(func_call->func, cg_context.get_effect_context(), &effect_accum);
  

diff --git a/src/Probabilities.cpp b/src/Probabilities.cpp
index 82b6ef0..a09e734 100644
--- a/src/Probabilities.cpp
+++ b/src/Probabilities.cpp
@@ -254,7 +254,7 @@ GroupProbElem::initialize(Probabilities *impl, const std::map<ProbName, int> pai
int val = (*i).second;
assert(val >= 0 && val <= 100);
ProbName pname = (*i).first;

•           std::string sname = impl->get_sname(pname);
  

•           // std::string sname = impl->get_sname(pname);
            assert(probs_[pname]);
            probs_[pname]->set_prob(val);
        }
  

diff --git a/src/StatementAssign.cpp b/src/StatementAssign.cpp
index 172613f..0dc6abd 100644
--- a/src/StatementAssign.cpp
+++ b/src/StatementAssign.cpp
@@ -330,8 +330,8 @@ StatementAssign::compound_to_binary_ops(eAssignOps op)
bool
StatementAssign::visit_facts(vector<const Fact*>& inputs, CGContext& cg_context) const
{

• vector<const Fact*> inputs_copy = inputs;
• // LHS and RHS can be evaludated in arbitrary order, try RHS first

• // vector<const Fact*> inputs_copy = inputs;
• // LHS and RHS can be evaluated in arbitrary order, try RHS first
  Effect running_eff_context(cg_context.get_effect_context());
  Effect rhs_accum, lhs_accum;
  CGContext rhs_cg_context(cg_context, running_eff_context, &rhs_accum);

diff --git a/src/StatementFor.cpp b/src/StatementFor.cpp
index a41fd5c..16f9883 100644
--- a/src/StatementFor.cpp
+++ b/src/StatementFor.cpp
@@ -165,7 +165,7 @@ StatementFor::make_iteration(CGContext& cg_context, StatementAssign*& init, Expr
assert(blk);

// save a copy of facts env and context

• vector<const Fact*> facts_copy = fm->global_facts;

• // vector<const Fact*> facts_copy = fm->global_facts;
  cg_context.get_effect_stm().clear();

  // Select the loop control variable, avoid volatile

cppcheck might run a bit faster with these deletions.