[PATCH] D23375: Add kfree( ) to MallocChecker.cpp

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:593
@@ -590,1 +592,3 @@
+      if (FunI == II_free || FunI == II_realloc ||
+	  FunI == II_reallocf || FunI == II_kfree)
         return true;
----------------
It looks like you are using tabs here. The LLVM style guide calls for spaces. http://llvm.org/docs/CodingStandards.html#use-spaces-instead-of-tabs

If you haven't seen it, the clang-format tool can help automatically format code to conform to LLVM's expected style.

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:950
@@ -945,3 +949,3 @@
   // Iterate over the constructor parameters.
-  for (const auto *CtorParam : CtorD->parameters()) {
+  for (const auto *CtorParam : CtorD->params()) {
 
----------------
It looks like this method has recently been renamed "parameters()" and so your patch doesn't compile. Are you using top of trunk from the llvm.org repo?

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1296
@@ -1291,2 +1295,3 @@
     case AF_IfNameIndex: os << "'if_nameindex()'"; return;
+    case AF_KMalloc: os <<"kmalloc()"; return;
     case AF_Alloca:
----------------
I think it would good to add a test that covers this code. There are analogous tests in test/Analysis/free.c -- that would be a good place for this added test.

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1309
@@ -1303,2 +1308,3 @@
     case AF_IfNameIndex: os << "'if_freenameindex()'"; return;
+    case AF_KMalloc: os << "kfree()"; return;
     case AF_Alloca:
----------------
Should there be a test for this case, as well? This would be exercised if code is allocated with kmalloc() and then freed in some way other than kfree (such as free() or delete). There are analogous tests in the MismatchedDeallocator test files.

================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2206
@@ -2198,2 +2205,3 @@
          isCMemFunction(FD, Ctx, AF_IfNameIndex,
-                        MemoryOperationKind::MOK_Free)))
+			MemoryOperationKind::MOK_Free) ||
+	 isCMemFunction(FD, Ctx, AF_KMalloc, MemoryOperationKind::MOK_Free)))
----------------
There are tabs on these two lines, as well.


https://reviews.llvm.org/D23375