Skip to content

Includes of system headers are never implicitly relative to the source file #282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions simplecpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2864,8 +2864,6 @@ static std::string openHeader(std::ifstream &f, const simplecpp::DUI &dui, const

if (systemheader) {
ret = openHeaderIncludePath(f, dui, header);
if (ret.empty())
return openHeaderRelative(f, sourcefile, header);
return ret;
}

Expand Down Expand Up @@ -2894,8 +2892,8 @@ static std::string getFileName(const std::map<std::string, simplecpp::TokenList
return s;
}

if (filedata.find(relativeFilename) != filedata.end())
return relativeFilename;
if (systemheader && filedata.find(header) != filedata.end())
return header;

return "";
}
Expand Down Expand Up @@ -3223,7 +3221,7 @@ void simplecpp::preprocess(simplecpp::TokenList &output, const simplecpp::TokenL

const Token * const inctok = inc2.cfront();

const bool systemheader = (inctok->op == '<');
const bool systemheader = (inctok->str()[0] == '<');
Copy link
Collaborator

@firewave firewave Dec 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This indicates that code above which also checks op for < might never be executed,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is inctok->op supposed to have any meaning here? By dumping out its value every time we reach this point in a run of the testrunner binary, I can see it's a null character in every case except in include7() and include8(), which are both testing the case where the argument to #include is derived by expanding a preprocessor macro; in these cases it holds a < character. So perhaps it's left over from the macro expansion? (Apologies, I'm new to the cppcheck innards, I'm not familiar with how it works.)

By chance, until I added my new test, these were also the only cases where the filespec in inctok->str() used angle brackets, so for the cases tested by testrunner so far, testing inctok->op was adequate. I agree this looks wrong, but I'm unsure whether the fix should be to look at inctok->str() instead, or whether to fix the initialisation of inctok->op.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial observation was based on a short look and was wrong. I am also not familiar in how it works.

op is set to < in that code above. Since with your change that is not used anymore that can be removed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An additional note. It seems op should only be set if it is a single character token. And if the token isn't already something like a name, number or comment. That wasn't done consistently though - I addressed that within #285.

const std::string header(realFilename(inctok->str().substr(1U, inctok->str().size() - 2U)));
std::string header2 = getFileName(filedata, rawtok->location.file(), header, dui, systemheader);
if (header2.empty()) {
Expand Down
17 changes: 17 additions & 0 deletions test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1653,6 +1653,22 @@ static void nestedInclude()
ASSERT_EQUALS("file0,1,include_nested_too_deeply,#include nested too deeply\n", toString(outputList));
}

static void systemInclude()
{
const char code[] = "#include <limits.h>\n";
std::vector<std::string> files;
simplecpp::TokenList rawtokens = makeTokenList(code,files,"local/limits.h");
std::map<std::string, simplecpp::TokenList*> filedata;
filedata["limits.h"] = nullptr;
filedata["local/limits.h"] = &rawtokens;

simplecpp::OutputList outputList;
simplecpp::TokenList tokens2(files);
simplecpp::preprocess(tokens2, rawtokens, files, filedata, simplecpp::DUI(), &outputList);

ASSERT_EQUALS("", toString(outputList));
}

static void multiline1()
{
const char code[] = "#define A \\\n"
Expand Down Expand Up @@ -2566,6 +2582,7 @@ int main(int argc, char **argv)
TEST_CASE(missingHeader2);
TEST_CASE(missingHeader3);
TEST_CASE(nestedInclude);
TEST_CASE(systemInclude);

TEST_CASE(nullDirective1);
TEST_CASE(nullDirective2);
Expand Down