kalibera / rchk

102 stars 10 forks source link

Puzzled about "results will be incomplete" #13

Closed zhanxw closed 5 years ago

zhanxw commented 5 years ago

I got the error: ignoring variable ret as it has address taken, results will be incomplete. But I don't understand why taking a pointer of SEXP (has address taken) can lead to incomplete results. Is there a reason for that?

The error:

Function impl_readTabixHeader [UP] ignoring variable ret as it has address taken, results will be incomplete

Relevant codes:

SEXP impl_readTabixHeader(SEXP arg_tabixFile) {
  SEXP ret = R_NilValue;

  std::vector<std::string> FLAG_tabixFile;
  extractStringArray(arg_tabixFile, &FLAG_tabixFile);
  TabixReader tr(FLAG_tabixFile[0]);
  if (!tr.good()) {
    REprintf("Cannot open specified tabix file: %s\n", FLAG_tabixFile[0].c_str());
    return ret;
  }

  std::vector<std::string> headers;
  stringTokenize(stringStrip(tr.getHeader()), "\n", &headers);

  storeResult(headers, &ret);
  return ret;
}
void storeResult(const std::vector<std::string>& in, SEXP* ret);

Environment:

docker image rhub/ubuntu-rchk (LLVM-4.0)

kalibera commented 5 years ago

This means "results of the analysis will be incomplete", so the analysis may miss some protect errors, in this case errors related to variable "ret" (it is much harder/too hard to track the data flow when pointers/addresses are used). In the CRAN rchk checks, I filter out incomplete reports as not to confuse users, but that of course comes with the price of possibly missing some true errors. In this case, the address of ret is taken in storeResult(headers, &ret)" If storeResult just returned ret as return value, the function could be analyzed by rchk (and it is perhaps also a better design if possible to have a return value instead of output argument).