[PATCH] D14014: Checker of proper vfork usage

[Yury Gribov via cfe-commits]

ygribov added inline comments.

================
Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:47
@@ +46,3 @@
+
+  bool isChildProcess(const ProgramStateRef State) const;
+
----------------
a.sidorin wrote:
> I think it's a good idea to make some functions static and/or move them out of class definition.
Right. Which one is preferred btw?

================
Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:97
@@ +96,3 @@
+
+    ASTContext &Ctx = C.getASTContext();
+    for (const char **id = ids; *id; ++id)
----------------
a.sidorin wrote:
> What about combination of std::transform + std::sort + std::binary_search?
I think I should use SmallSet here as suggested by Devin.

================
Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:149
@@ +148,3 @@
+
+  // see if it's an ordinary assignment
+  do {
----------------
a.sidorin wrote:
> You can use early return to escape do{}.
In this particular case - yes. But what's wrong about original do-while-false? I thought it's a common idiom...

================
Comment at: lib/StaticAnalyzer/Checkers/VforkChecker.cpp:196
@@ +195,3 @@
+  SVal VforkRetVal = Call.getReturnValue();
+  SymbolRef Sym = VforkRetVal.getAsSymbol();
+  Optional<DefinedOrUnknownSVal> DVal =
----------------
a.sidorin wrote:
> Is check for non-concrete value is really required?
This may be remains of old code, I'll re-check.


Repository:
  rL LLVM

http://reviews.llvm.org/D14014