From: Anna Zaks <ganna@apple.com>
Date: Thu, 4 Aug 2011 00:31:38 +0000 (+0000)
Subject: KeychainAPI checker: refactor to use early exit.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=08551b575396ec89411a4e416d27fd7056ceaa9b;p=clang

KeychainAPI checker: refactor to use early exit.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@136852 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
index 7fd0e51607..d173c0f1c5 100644
--- a/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
@@ -116,33 +116,34 @@ void MacOSKeychainAPIChecker::checkPreStmt(const CallExpr *CE,
 
   // If a value has been freed, remove from the list.
   unsigned idx = getDeallocatingFunctionParam(funName);
-  if (idx != InvalidParamVal) {
-    const Expr *ArgExpr = CE->getArg(idx);
-    const MemRegion *Arg = State->getSVal(ArgExpr).getAsRegion();
-    if (!Arg)
+  if (idx == InvalidParamVal)
+    return;
+
+  const Expr *ArgExpr = CE->getArg(idx);
+  const MemRegion *Arg = State->getSVal(ArgExpr).getAsRegion();
+  if (!Arg)
+    return;
+
+  // If trying to free data which has not been allocated yet, report as bug.
+  if (State->get<AllocatedData>(Arg) == 0) {
+    // It is possible that this is a false positive - the argument might
+    // have entered as an enclosing function parameter.
+    if (isEnclosingFunctionParam(ArgExpr))
       return;
 
-    // If trying to free data which has not been allocated yet, report as bug.
-    if (State->get<AllocatedData>(Arg) == 0) {
-      // It is possible that this is a false positive - the argument might
-      // have entered as an enclosing function parameter.
-      if (isEnclosingFunctionParam(ArgExpr))
-        return;
-
-      ExplodedNode *N = C.generateNode(State);
-      if (!N)
-        return;
-      initBugType();
-      RangedBugReport *Report = new RangedBugReport(*BT,
-          "Trying to free data which has not been allocated.", N);
-      Report->addRange(ArgExpr->getSourceRange());
-      C.EmitReport(Report);
-    }
-
-    // Continue exploring from the new state.
-    State = State->remove<AllocatedData>(Arg);
-    C.addTransition(State);
+    ExplodedNode *N = C.generateNode(State);
+    if (!N)
+      return;
+    initBugType();
+    RangedBugReport *Report = new RangedBugReport(*BT,
+        "Trying to free data which has not been allocated.", N);
+    Report->addRange(ArgExpr->getSourceRange());
+    C.EmitReport(Report);
   }
+
+  // Continue exploring from the new state.
+  State = State->remove<AllocatedData>(Arg);
+  C.addTransition(State);
 }
 
 void MacOSKeychainAPIChecker::checkPostStmt(const CallExpr *CE,
@@ -162,32 +163,33 @@ void MacOSKeychainAPIChecker::checkPostStmt(const CallExpr *CE,
 
   // If a value has been allocated, add it to the set for tracking.
   unsigned idx = getAllocatingFunctionParam(funName);
-  if (idx != InvalidParamVal) {
-    SVal Arg = State->getSVal(CE->getArg(idx));
-    if (const loc::MemRegionVal *X = dyn_cast<loc::MemRegionVal>(&Arg)) {
-      // Add the symbolic value, which represents the location of the allocated
-      // data, to the set.
-      const MemRegion *V = SM.Retrieve(State->getStore(), *X).getAsRegion();
-      // If this is not a region, it can be:
-      //  - unknown (cannot reason about it)
-      //  - undefined (already reported by other checker)
-      //  - constant (null - should not be tracked, other - report a warning?)
-      //  - goto (should be reported by other checker)
-      if (!V)
-        return;
-
-      State = State->set<AllocatedData>(V, AllocationInfo(CE->getArg(idx)));
-
-      // We only need to track the value if the function returned noErr(0), so
-      // bind the return value of the function to 0.
-      SValBuilder &Builder = C.getSValBuilder();
-      SVal ZeroVal = Builder.makeZeroVal(Builder.getContext().CharTy);
-      State = State->BindExpr(CE, ZeroVal);
-      assert(State);
-
-      // Proceed from the new state.
-      C.addTransition(State);
-    }
+  if (idx == InvalidParamVal)
+    return;
+
+  SVal Arg = State->getSVal(CE->getArg(idx));
+  if (const loc::MemRegionVal *X = dyn_cast<loc::MemRegionVal>(&Arg)) {
+    // Add the symbolic value, which represents the location of the allocated
+    // data, to the set.
+    const MemRegion *V = SM.Retrieve(State->getStore(), *X).getAsRegion();
+    // If this is not a region, it can be:
+    //  - unknown (cannot reason about it)
+    //  - undefined (already reported by other checker)
+    //  - constant (null - should not be tracked, other - report a warning?)
+    //  - goto (should be reported by other checker)
+    if (!V)
+      return;
+
+    State = State->set<AllocatedData>(V, AllocationInfo(CE->getArg(idx)));
+
+    // We only need to track the value if the function returned noErr(0), so
+    // bind the return value of the function to 0.
+    SValBuilder &Builder = C.getSValBuilder();
+    SVal ZeroVal = Builder.makeZeroVal(Builder.getContext().CharTy);
+    State = State->BindExpr(CE, ZeroVal);
+    assert(State);
+
+    // Proceed from the new state.
+    C.addTransition(State);
   }
 }