From d9b278de666c03c93ee4c27a341554a0bc6c3148 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Tue, 26 Nov 2024 11:45:55 +0100 Subject: [PATCH 1/8] C++: Promote `cpp/guarded-free` --- .../Best Practices/GuardedFree.cpp | 0 .../Best Practices/GuardedFree.qhelp | 18 ++++++++++++++---- .../Best Practices/GuardedFree.ql | 1 - .../GuardedFree/GuardedFree.expected | 0 .../GuardedFree/GuardedFree.qlref | 0 .../Best Practices/GuardedFree/test.cpp | 0 6 files changed, 14 insertions(+), 5 deletions(-) rename cpp/ql/src/{experimental => }/Best Practices/GuardedFree.cpp (100%) rename cpp/ql/src/{experimental => }/Best Practices/GuardedFree.qhelp (66%) rename cpp/ql/src/{experimental => }/Best Practices/GuardedFree.ql (98%) rename cpp/ql/test/{experimental => }/query-tests/Best Practices/GuardedFree/GuardedFree.expected (100%) rename cpp/ql/test/{experimental => }/query-tests/Best Practices/GuardedFree/GuardedFree.qlref (100%) rename cpp/ql/test/{experimental => }/query-tests/Best Practices/GuardedFree/test.cpp (100%) diff --git a/cpp/ql/src/experimental/Best Practices/GuardedFree.cpp b/cpp/ql/src/Best Practices/GuardedFree.cpp similarity index 100% rename from cpp/ql/src/experimental/Best Practices/GuardedFree.cpp rename to cpp/ql/src/Best Practices/GuardedFree.cpp diff --git a/cpp/ql/src/experimental/Best Practices/GuardedFree.qhelp b/cpp/ql/src/Best Practices/GuardedFree.qhelp similarity index 66% rename from cpp/ql/src/experimental/Best Practices/GuardedFree.qhelp rename to cpp/ql/src/Best Practices/GuardedFree.qhelp index 77fdd4670008..ba78749bef0a 100644 --- a/cpp/ql/src/experimental/Best Practices/GuardedFree.qhelp +++ b/cpp/ql/src/Best Practices/GuardedFree.qhelp @@ -1,18 +1,28 @@ - - + + +

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.

+

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

 + + +

In this example the condition checking the value of foo can be deleted. + +

  • 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/Best Practices/GuardedFree.ql similarity index 98% rename from cpp/ql/src/experimental/Best Practices/GuardedFree.ql rename to cpp/ql/src/Best Practices/GuardedFree.ql index 66c7a31111b5..ea81a7158282 100644 --- a/cpp/ql/src/experimental/Best Practices/GuardedFree.ql +++ b/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/test/experimental/query-tests/Best Practices/GuardedFree/GuardedFree.expected b/cpp/ql/test/query-tests/Best Practices/GuardedFree/GuardedFree.expected similarity index 100% rename from cpp/ql/test/experimental/query-tests/Best Practices/GuardedFree/GuardedFree.expected rename to cpp/ql/test/query-tests/Best Practices/GuardedFree/GuardedFree.expected diff --git a/cpp/ql/test/experimental/query-tests/Best Practices/GuardedFree/GuardedFree.qlref b/cpp/ql/test/query-tests/Best Practices/GuardedFree/GuardedFree.qlref similarity index 100% rename from cpp/ql/test/experimental/query-tests/Best Practices/GuardedFree/GuardedFree.qlref rename to cpp/ql/test/query-tests/Best Practices/GuardedFree/GuardedFree.qlref diff --git a/cpp/ql/test/experimental/query-tests/Best Practices/GuardedFree/test.cpp b/cpp/ql/test/query-tests/Best Practices/GuardedFree/test.cpp similarity index 100% rename from cpp/ql/test/experimental/query-tests/Best Practices/GuardedFree/test.cpp rename to cpp/ql/test/query-tests/Best Practices/GuardedFree/test.cpp From e1f70a0dec525d020bd9a6ef0bc4aa51f94a0a16 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Tue, 26 Nov 2024 13:50:09 +0100 Subject: [PATCH 2/8] C++: Add missing `

    ` to qlhelp --- cpp/ql/src/Best Practices/GuardedFree.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/Best Practices/GuardedFree.qhelp b/cpp/ql/src/Best Practices/GuardedFree.qhelp index ba78749bef0a..15e78197cfd3 100644 --- a/cpp/ql/src/Best Practices/GuardedFree.qhelp +++ b/cpp/ql/src/Best Practices/GuardedFree.qhelp @@ -14,7 +14,7 @@ -

    In this example the condition checking the value of foo can be deleted. +

    In this example the condition checking the value of foo can be deleted.

     From fc6c327ab7f9a561c9cdd73a3e03c800ac6ae8dd Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Tue, 26 Nov 2024 13:55:30 +0100 Subject: [PATCH 3/8] C++: Add change note --- cpp/ql/src/change-notes/2014-11-26-guarded-free.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 cpp/ql/src/change-notes/2014-11-26-guarded-free.md 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 new file mode 100644 index 000000000000..4280025a04f6 --- /dev/null +++ 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). From 6aa7c93af2977184c0b2e1d83ac3c3c74f0deb9f Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Tue, 26 Nov 2024 13:58:54 +0100 Subject: [PATCH 4/8] C++: More qlhelp fixes --- cpp/ql/src/Best Practices/GuardedFree.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/Best Practices/GuardedFree.qhelp b/cpp/ql/src/Best Practices/GuardedFree.qhelp index 15e78197cfd3..6327354aded6 100644 --- a/cpp/ql/src/Best Practices/GuardedFree.qhelp +++ b/cpp/ql/src/Best Practices/GuardedFree.qhelp @@ -1,7 +1,7 @@ - +

    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.

    From 8d591596917a47d1de65c1cf06391418288a03c2 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema Date: Tue, 26 Nov 2024 15:35:52 +0100 Subject: [PATCH 5/8] C++: Fix qlref file --- .../query-tests/Best Practices/GuardedFree/GuardedFree.qlref | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/test/query-tests/Best Practices/GuardedFree/GuardedFree.qlref b/cpp/ql/test/query-tests/Best Practices/GuardedFree/GuardedFree.qlref index e28bdae39916..d64671f08c33 100644 --- a/cpp/ql/test/query-tests/Best Practices/GuardedFree/GuardedFree.qlref +++ b/cpp/ql/test/query-tests/Best Practices/GuardedFree/GuardedFree.qlref @@ -1 +1 @@ -experimental/Best Practices/GuardedFree.ql +Best Practices/GuardedFree.ql From 088a3ef15c88e8552d2eb7654c37c14839143fe2 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema <93738568+jketema@users.noreply.github.com> Date: Thu, 28 Nov 2024 16:35:39 +0100 Subject: [PATCH 6/8] Update cpp/ql/src/Best Practices/GuardedFree.qhelp Co-authored-by: mc <42146119+mchammer01@users.noreply.github.com> --- cpp/ql/src/Best Practices/GuardedFree.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/Best Practices/GuardedFree.qhelp b/cpp/ql/src/Best Practices/GuardedFree.qhelp index 6327354aded6..32364d4d5f78 100644 --- a/cpp/ql/src/Best Practices/GuardedFree.qhelp +++ b/cpp/ql/src/Best Practices/GuardedFree.qhelp @@ -21,7 +21,7 @@
  • The Open Group Base Specifications Issue 7, 2018 Edition: - free - free allocated memory + free - free allocated memory.
    • From f9d9f9ba6211842beacab29f5ec15fe3d8e378d4 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema <93738568+jketema@users.noreply.github.com> Date: Thu, 28 Nov 2024 16:36:04 +0100 Subject: [PATCH 7/8] Update cpp/ql/src/Best Practices/GuardedFree.qhelp Co-authored-by: mc <42146119+mchammer01@users.noreply.github.com> --- cpp/ql/src/Best Practices/GuardedFree.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/Best Practices/GuardedFree.qhelp b/cpp/ql/src/Best Practices/GuardedFree.qhelp index 32364d4d5f78..ae41620528ff 100644 --- a/cpp/ql/src/Best Practices/GuardedFree.qhelp +++ b/cpp/ql/src/Best Practices/GuardedFree.qhelp @@ -14,7 +14,7 @@ -

    In this example the condition checking the value of foo can be deleted.

    +

    In this example, the condition checking the value of foo can be deleted.

     From 6d37efc0d8dd2e89cd45ae6f7846ea203167de66 Mon Sep 17 00:00:00 2001 From: Jeroen Ketema <93738568+jketema@users.noreply.github.com> Date: Thu, 28 Nov 2024 16:36:13 +0100 Subject: [PATCH 8/8] Update cpp/ql/src/Best Practices/GuardedFree.qhelp Co-authored-by: mc <42146119+mchammer01@users.noreply.github.com> --- cpp/ql/src/Best Practices/GuardedFree.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/Best Practices/GuardedFree.qhelp b/cpp/ql/src/Best Practices/GuardedFree.qhelp index ae41620528ff..c2dc5cb36d78 100644 --- a/cpp/ql/src/Best Practices/GuardedFree.qhelp +++ b/cpp/ql/src/Best Practices/GuardedFree.qhelp @@ -4,7 +4,7 @@ -

    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.

    +

    The free 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 free. As such, these guards may hinder performance and readability.