Merge pull request #18111 from jketema/guarded-free

C++: Promote `cpp/guarded-free` out of experimental
github · Nov 29, 2024 · bcb7901 · bcb7901
2 parents 47487cc + 6d37efc
commit bcb7901
Show file tree

Hide file tree

Showing 8 changed files with 20 additions and 7 deletions.
diff --git a/...perimental/Best Practices/GuardedFree.cpp → cpp/ql/src/Best Practices/GuardedFree.cpp b/...perimental/Best Practices/GuardedFree.cpp → cpp/ql/src/Best Practices/GuardedFree.cpp
diff --git a/...rimental/Best Practices/GuardedFree.qhelp → cpp/ql/src/Best Practices/GuardedFree.qhelp b/...rimental/Best Practices/GuardedFree.qhelp → cpp/ql/src/Best Practices/GuardedFree.qhelp
@@ -1,18 +1,28 @@
-<!DOCTYPE qhelp SYSTEM "qhelp.dtd">
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
 <qhelp>
+
 <overview>
-<p>The <code>free</code> 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 <code>free</code>. As such, these guards may hinder performance and readability.</p>
+<p>The <code>free</code> function, which deallocates heap memory, may accept a NULL pointer and take no action. Therefore, it is unnecessary to check the argument for the value of NULL before a function call to <code>free</code>. As such, these guards may hinder performance and readability.</p>
 </overview>
+
 <recommendation>
-<p>A function call to <code>free</code> should not depend upon the value of its argument. Delete the <code>if</code> condition preceeding a function call to <code>free</code> when its only purpose is to check the value of the pointer to be freed.</p>
+<p>A function call to <code>free</code> should not depend upon the value of its argument. Delete the condition preceding a function call to <code>free</code> when its only purpose is to check the value of the pointer to be freed.</p>
 </recommendation>
+
 <example>
 <sample src = "GuardedFree.cpp" />
+
+<p>In this example, the condition checking the value of <code>foo</code> can be deleted.</p>
+
 </example>
+
 <references>
 <li>
     The Open Group Base Specifications Issue 7, 2018 Edition:
-    <a href="https://pubs.opengroup.org/onlinepubs/9699919799/functions/free.html">free - free allocated memory</a>
+    <a href="https://pubs.opengroup.org/onlinepubs/9699919799/functions/free.html">free - free allocated memory</a>.
 </li>
 </references>
-</qhelp>
+
+</qhelp>
diff --git a/...xperimental/Best Practices/GuardedFree.ql → cpp/ql/src/Best Practices/GuardedFree.ql b/...xperimental/Best Practices/GuardedFree.ql → cpp/ql/src/Best Practices/GuardedFree.ql
@@ -8,7 +8,6 @@
  * @id cpp/guarded-free
  * @tags maintainability
  *       readability
- *       experimental
  */
 
 import cpp

diff --git a/cpp/ql/src/change-notes/2014-11-26-guarded-free.md b/cpp/ql/src/change-notes/2014-11-26-guarded-free.md
@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new high-precision quality query, `cpp/guarded-free`, which detects useless NULL pointer checks before calls to `free`. A variation of this query was originally contributed as an [experimental query by @mario-campos](https://github.com/github/codeql/pull/16331).
diff --git a/cpp/ql/test/experimental/query-tests/Best Practices/GuardedFree/GuardedFree.qlref b/cpp/ql/test/experimental/query-tests/Best Practices/GuardedFree/GuardedFree.qlref
diff --git a/...ractices/GuardedFree/GuardedFree.expected → ...ractices/GuardedFree/GuardedFree.expected b/...ractices/GuardedFree/GuardedFree.expected → ...ractices/GuardedFree/GuardedFree.expected
diff --git a/cpp/ql/test/query-tests/Best Practices/GuardedFree/GuardedFree.qlref b/cpp/ql/test/query-tests/Best Practices/GuardedFree/GuardedFree.qlref
@@ -0,0 +1 @@
+Best Practices/GuardedFree.ql
diff --git a/...tests/Best Practices/GuardedFree/test.cpp → ...tests/Best Practices/GuardedFree/test.cpp b/...tests/Best Practices/GuardedFree/test.cpp → ...tests/Best Practices/GuardedFree/test.cpp