[PATCH] D14014: Checker of proper vfork usage
Aleksei Sidorin via cfe-commits
cfe-commits at lists.llvm.org
Sat Oct 24 05:33:35 PDT 2015
a.sidorin added a comment.
I put some suggestions in inline comments.
================
Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:47
@@ +46,3 @@
+
+ bool isChildProcess(const ProgramStateRef State) const;
+
----------------
I think it's a good idea to make some functions static and/or move them out of class definition.
================
Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:97
@@ +96,3 @@
+
+ ASTContext &Ctx = C.getASTContext();
+ for (const char **id = ids; *id; ++id)
----------------
What about combination of std::transform + std::sort + std::binary_search?
================
Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:136
@@ +135,3 @@
+ const DeclStmt *PD = dyn_cast_or_null<DeclStmt>(P);
+ if (!PD)
+ break;
----------------
```
if (PD && PD->isSingleDecl()) {
if (const VarDecl *D = dyn_cast<VarDecl>(PD->getSingleDecl())
return D;
```
================
Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:149
@@ +148,3 @@
+
+ // see if it's an ordinary assignment
+ do {
----------------
You can use early return to escape do{}.
================
Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:196
@@ +195,3 @@
+ SVal VforkRetVal = Call.getReturnValue();
+ SymbolRef Sym = VforkRetVal.getAsSymbol();
+ Optional<DefinedOrUnknownSVal> DVal =
----------------
Is check for non-concrete value is really required?
================
Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:252
@@ +251,3 @@
+ ProgramStateRef State = C.getState();
+ if (!isChildProcess(State))
+ return;
----------------
```
if (is...)
report...
```
Repository:
rL LLVM
http://reviews.llvm.org/D14014
More information about the cfe-commits
mailing list