[PATCH] D72705: [analyzer] Added new checker 'alpha.unix.ErrorReturn'.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

After running the checker I could observe the following problems:

  time_t now = time(NULL);
  if (now > 0) { ... }

Here only `now == EOF` would be correct for the checker, so this case is reported (false positive). It may be better if the checker finds any "now > //x//" where //x// is non-negative. This can be used for any function that returns an integer value (not pointer) and EOF is the error return code.

  c = fgetc(fd);
  if (c == '+' || c == '*' || c == '|' || c == '>' || c == '@' || c == EOF || c == '\n') { ... }

The first `c == '+'` is found by the checker and reported as false positive (the later `c == EOF` is not found). Such a case can be found if the checker can collect expressions that are separated by `||` or `&&` and the symbol to check occurs in these and there is only a simple comparison.

The checker can find places where the return value is tested for error (mostly early-return cases), not where the return value is tested for a valid value (that may be a subset of all non-error values). And the test for error or valid value should be in a single statement, not in nested `if`s for example.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72705/new/

https://reviews.llvm.org/D72705