a.sidorin added a comment.
I like this refactoring. I wrote some things that are not clear for me inline.
================
Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:107
+ void TryXNULock(const CallEvent &Call, CheckerContext &C) const;
+ void AcquireLockAux(const CallEvent &Call, CheckerContext &C, unsigned ArgNo,
+ SVal lock, bool isTryLock,
----------------
In the code below `lock` is always `Call.getSVal(ArgNo)`. Should we remove it
and put SVal acquisition into callees?
================
Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:108
+ void AcquireLockAux(const CallEvent &Call, CheckerContext &C, unsigned ArgNo,
+ SVal lock, bool isTryLock,
+ enum LockingSemantics semantics) const;
----------------
According to naming conventions , it should be Lock and IsTryLock. But I'm not
sure that this corresponds to the file's code style so I do not insist.
================
Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:188
+ if (Call.isCalled(I.Name))
+ (this->*I.Callback)(Call, C);
+ }
----------------
break?
================
Comment at: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp:262
+ CheckerContext &C) const {
+ AcquireLockAux(Call, C, 0, Call.getArgSVal(0), false, PthreadSemantics);
+}
----------------
Could you add comments describing what arguments should do? I.e. `/* ArgNo= */
0, /* Lock= */...? It's hard to remember what do these parameters correspond to.
https://reviews.llvm.org/D37809
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
