diff --git a/cpp/ql/src/experimental/Best Practices/GuardedFree.cpp b/cpp/ql/src/experimental/Best Practices/GuardedFree.cpp new file mode 100644 index 000000000000..5242e3da8f5e --- /dev/null +++ b/cpp/ql/src/experimental/Best Practices/GuardedFree.cpp @@ -0,0 +1,11 @@ +void test() +{ + char *foo = malloc(100); + + // BAD + if (foo) + free(foo); + + // GOOD + free(foo); +} \ No newline at end of file diff --git a/cpp/ql/src/experimental/Best Practices/GuardedFree.qhelp b/cpp/ql/src/experimental/Best Practices/GuardedFree.qhelp new file mode 100644 index 000000000000..77fdd4670008 --- /dev/null +++ b/cpp/ql/src/experimental/Best Practices/GuardedFree.qhelp @@ -0,0 +1,18 @@ + + + +

The free function, which deallocates heap memory, may accept a NULL pointer and take no action. Therefore, it is unnecessary to check its argument for the value of NULL before a function call to free. As such, these guards may hinder performance and readability.

+
+ +

A function call to free should not depend upon the value of its argument. Delete the if condition preceeding a function call to free when its only purpose is to check the value of the pointer to be freed.

+
+ + + + +
  • + The Open Group Base Specifications Issue 7, 2018 Edition: + free - free allocated memory +
  • +
    +
    \ No newline at end of file diff --git a/cpp/ql/src/experimental/Best Practices/GuardedFree.ql b/cpp/ql/src/experimental/Best Practices/GuardedFree.ql new file mode 100644 index 000000000000..2d504d9bc057 --- /dev/null +++ b/cpp/ql/src/experimental/Best Practices/GuardedFree.ql @@ -0,0 +1,26 @@ +/** + * @name Guarded Free + * @description NULL-condition guards before function calls to the memory-deallocation + * function free(3) are unnecessary, because passing NULL to free(3) is a no-op. + * @kind problem + * @problem.severity recommendation + * @precision very-high + * @id cpp/guarded-free + * @tags maintainability + * readability + * experimental + */ + +import cpp +import semmle.code.cpp.controlflow.Guards + +class FreeCall extends FunctionCall { + FreeCall() { this.getTarget().hasGlobalName("free") } +} + +from GuardCondition gc, FreeCall fc, Variable v, BasicBlock bb +where + gc.ensuresEq(v.getAnAccess(), 0, bb, false) and + fc.getArgument(0) = v.getAnAccess() and + bb = fc.getEnclosingStmt() +select gc, "unnecessary NULL check before call to $@", fc, "free"