Szelethus added a comment.
A few nits inline, otherwise the patch is awesome, thank you!!
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:60-61
private:
- void Fopen(CheckerContext &C, const CallExpr *CE) const;
- void Tmpfile(CheckerContext &C, const CallExpr *CE) const;
- void Fclose(CheckerContext &C, const CallExpr *CE) const;
- void Fread(CheckerContext &C, const CallExpr *CE) const;
- void Fwrite(CheckerContext &C, const CallExpr *CE) const;
- void Fseek(CheckerContext &C, const CallExpr *CE) const;
- void Ftell(CheckerContext &C, const CallExpr *CE) const;
- void Rewind(CheckerContext &C, const CallExpr *CE) const;
- void Fgetpos(CheckerContext &C, const CallExpr *CE) const;
- void Fsetpos(CheckerContext &C, const CallExpr *CE) const;
- void Clearerr(CheckerContext &C, const CallExpr *CE) const;
- void Feof(CheckerContext &C, const CallExpr *CE) const;
- void Ferror(CheckerContext &C, const CallExpr *CE) const;
- void Fileno(CheckerContext &C, const CallExpr *CE) const;
-
- void OpenFileAux(CheckerContext &C, const CallExpr *CE) const;
-
- ProgramStateRef CheckNullStream(SVal SV, ProgramStateRef state,
- CheckerContext &C) const;
- ProgramStateRef CheckDoubleClose(const CallExpr *CE, ProgramStateRef state,
- CheckerContext &C) const;
+ typedef void (StreamChecker::*FnCheck)(const CallEvent &,
+ CheckerContext &) const;
+
----------------
Prefer using. When I wrote D68165, I spent about 10 minutes figuring out how to
do it... Ah, the joys of the C++ syntax.
```lang=c++
using FnCheck = void (StreamChecker::*)(const CallEvent &,
CheckerContext &) const;
```
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:109-110
bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
+ if (!Call.isGlobalCFunction())
+ return false;
+
----------------
Isn't this redundant with my other inline about parameter types?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:127-131
+ for (auto P : Call.parameters()) {
+ QualType T = P->getType();
+ if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
+ return nullptr;
+ }
----------------
I'm not sure why we need this, is it true that *all* stream related functions
return a pointer or a numerical value? Are we actually checking whether this
really is a library function? If so, this looks pretty arbitrary.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:156
+void StreamChecker::evalFread(const CallEvent &Call, CheckerContext &C) const {
+ (void)CheckNullStream(Call.getArgSVal(3), C);
}
----------------
Why the need for `(void)`? `CheckNullSteam` doesn't seem to have an
`LLVM_NODISCARD` attribute.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67706/new/
https://reviews.llvm.org/D67706
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
