gamesh411 added a comment.
Great job, this seems to be progressing nicely! please see my comments inline.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:38
+ // See if the result value from the system function (to check) is checked for
+ // error after a branch condition. 'Value' contains the (mostly conjured)
+ // symbolic value of the function call. 'RetTy' is the return type of the
----------------
Value is not the name of the parameter, maybe a refactoring missed this comment.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:77
+// This should work with any type of null value.
+// FIXME: Is this different from the ValueErrorResultChecker with 0 as value?
+class NullErrorResultChecker : public CheckForErrorResultChecker {
----------------
I think the current implementation is slightly different. The
ValueErrorResultChecker uses symbolic evaluation of the equation with a
ConcreteInt 0 value, while the ProgramState::isNull checks if the value is a
constant and if it is not a zero constant (also calls further to
ConstraintManager::isNull).
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:158
+// value < 0" check too.
+class NegativeOrEofErrorResultChecker : public CheckForErrorResultChecker {
+public:
----------------
Maybe only checking for negative value is enough? My gut feeling is that the
equality check is redundant. I would argue:
let A be: Value is negative
let B be: Value is equal to -1
since B implies A, (A or B) can be simplified to just A.
This presupposes that every time B can reasoned about A can be too.
This seems to be the case for here as well, as the definiteness of being equal
to -1 is stronger than being lower than 0.
================
Comment at: clang/test/Analysis/error-return.c:270
+
+These are OK if not checked:
+
----------------
just for the sake of completeness; mention all the cases where error checking
is deemed unnecessary as per ERR-33-EX1
https://wiki.sei.cmu.edu/confluence/display/c/ERR33-C.+Detect+and+handle+standard+library+errors#ERR33-C.Detectandhandlestandardlibraryerrors-Exceptions
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71510/new/
https://reviews.llvm.org/D71510
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
