Skip to content

Commit

Permalink
Detect C functions that can be declared with static linkage (#7127)
Browse files Browse the repository at this point in the history
Currently limited to C as I assume the anonymous namespace should be
favored for C++.
  • Loading branch information
mptre authored Jan 10, 2025
1 parent 27a4500 commit 8b29139
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 14 deletions.
39 changes: 31 additions & 8 deletions lib/checkunusedfunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ void CheckUnusedFunctions::parseTokens(const Tokenizer &tokenizer, const Setting

if (!usage.lineNumber)
usage.lineNumber = func->token->linenr();
usage.isC = func->token->isC();
usage.isStatic = func->isStatic();

// TODO: why always overwrite this but not the filename and line?
usage.fileIndex = func->token->fileIndex();
Expand Down Expand Up @@ -337,6 +339,23 @@ static bool isOperatorFunction(const std::string & funcName)
return std::find(additionalOperators.cbegin(), additionalOperators.cend(), funcName.substr(operatorPrefix.length())) != additionalOperators.cend();
}

static void staticFunctionError(ErrorLogger& errorLogger,
const std::string &filename,
unsigned int fileIndex,
unsigned int lineNumber,
const std::string &funcname)
{
std::list<ErrorMessage::FileLocation> locationList;
if (!filename.empty()) {
locationList.emplace_back(filename, lineNumber, 0);
locationList.back().fileIndex = fileIndex;
}

const ErrorMessage errmsg(std::move(locationList), emptyString, Severity::style, "$symbol:" + funcname + "\nThe function '$symbol' should have static linkage since it is not used outside of its translation unit.", "staticFunction", Certainty::normal);
errorLogger.reportErr(errmsg);
}


#define logChecker(id) \
do { \
const ErrorMessage errmsg({}, nullptr, Severity::internal, "logChecker", (id), CWE(0U), Certainty::normal); \
Expand All @@ -349,6 +368,7 @@ bool CheckUnusedFunctions::check(const Settings& settings, ErrorLogger& errorLog

using ErrorParams = std::tuple<std::string, unsigned int, unsigned int, std::string>;
std::vector<ErrorParams> errors; // ensure well-defined order
std::vector<ErrorParams> staticFunctionErrors;

for (auto it = mFunctions.cbegin(); it != mFunctions.cend(); ++it) {
const FunctionUsage &func = it->second;
Expand All @@ -363,19 +383,22 @@ bool CheckUnusedFunctions::check(const Settings& settings, ErrorLogger& errorLog
if (func.filename != "+")
filename = func.filename;
errors.emplace_back(filename, func.fileIndex, func.lineNumber, it->first);
} else if (!func.usedOtherFile) {
/** @todo add error message "function is only used in <file> it can be static" */
/*
std::ostringstream errmsg;
errmsg << "The function '" << it->first << "' is only used in the file it was declared in so it should have local linkage.";
mErrorLogger->reportErr( errmsg.str() );
errors = true;
*/
} else if (func.isC && !func.isStatic && !func.usedOtherFile) {
std::string filename;
if (func.filename != "+")
filename = func.filename;
staticFunctionErrors.emplace_back(filename, func.fileIndex, func.lineNumber, it->first);
}
}

std::sort(errors.begin(), errors.end());
for (const auto& e : errors)
unusedFunctionError(errorLogger, std::get<0>(e), std::get<1>(e), std::get<2>(e), std::get<3>(e));

std::sort(staticFunctionErrors.begin(), staticFunctionErrors.end());
for (const auto& e : staticFunctionErrors)
staticFunctionError(errorLogger, std::get<0>(e), std::get<1>(e), std::get<2>(e), std::get<3>(e));

return !errors.empty();
}

Expand Down
2 changes: 2 additions & 0 deletions lib/checkunusedfunctions.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ class CPPCHECKLIB CheckUnusedFunctions {
unsigned int fileIndex{};
bool usedSameFile{};
bool usedOtherFile{};
bool isC{};
bool isStatic{};
};

std::unordered_map<std::string, FunctionUsage> mFunctions;
Expand Down
2 changes: 1 addition & 1 deletion samples/AssignmentAddressToInteger/bad.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
int foo(int *p)
static int foo(int *p)
{
int a = p;
return a + 4;
Expand Down
2 changes: 1 addition & 1 deletion samples/AssignmentAddressToInteger/good.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
int* foo(int *p)
static int* foo(int *p)
{
return p + 4;
}
Expand Down
2 changes: 1 addition & 1 deletion samples/autoVariables/bad.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
void foo(int **a)
static void foo(int **a)
{
int b = 1;
*a = &b;
Expand Down
2 changes: 1 addition & 1 deletion samples/autoVariables/good.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
void foo(int **a)
static void foo(int **a)
{
int b = 1;
**a = b;
Expand Down
2 changes: 1 addition & 1 deletion samples/incorrectLogicOperator/bad.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

void foo(int x) {
static void foo(int x) {
if (x >= 0 || x <= 10) {}
}

Expand Down
2 changes: 1 addition & 1 deletion samples/incorrectLogicOperator/good.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

void foo(int x) {
static void foo(int x) {
if (x >= 0 && x <= 10) {}
}

Expand Down
16 changes: 16 additions & 0 deletions test/testunusedfunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class TestUnusedFunctions : public TestFixture {
TEST_CASE(attributeCleanup);
TEST_CASE(attributeUnused);
TEST_CASE(attributeMaybeUnused);
TEST_CASE(staticFunction);
}

#define check(...) check_(__FILE__, __LINE__, __VA_ARGS__)
Expand Down Expand Up @@ -820,6 +821,21 @@ class TestUnusedFunctions : public TestFixture {
check("[[maybe_unused]] void f() {}\n");
ASSERT_EQUALS("", errout_str());
}

void staticFunction()
{
check("void f(void) {}\n"
"int main() {\n"
" f();\n"
"}\n");
ASSERT_EQUALS("", errout_str());

check("void f(void) {}\n"
"int main() {\n"
" f();\n"
"}\n", Platform::Type::Native, nullptr, false);
ASSERT_EQUALS("[test.c:1]: (style) The function 'f' should have static linkage since it is not used outside of its translation unit.\n", errout_str());
}
};

REGISTER_TEST(TestUnusedFunctions)

0 comments on commit 8b29139

Please sign in to comment.