C++: Promote cpp/guarded-free out of experimental

[jketema]

Pull Request checklist

All query authors

• A change note is added if necessary. See the documentation in this repository.
• All new queries have appropriate .qhelp. See the documentation in this repository.
• QL tests are added if necessary. See Testing custom queries in the GitHub documentation.
• New and changed queries have correct query metadata. See the documentation in this repository.

Internal query authors only

• Autofixes generated based on these changes are valid, only needed if this PR makes significant changes to .ql, .qll, or .qhelp files. See the documentation (internal access required).
• Changes are validated at scale (internal access required).
• Adding a new query? Consider also adding the query to autofix.

[github-actions]

QHelp previews:

cpp/ql/src/Best Practices/GuardedFree.qhelp

Guarded Free

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.

Recommendation

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.

Example

void test()
{
    char *foo = malloc(100);

    // BAD
    if (foo)          
        free(foo);

    // GOOD
    free(foo);
}

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

References

• The Open Group Base Specifications Issue 7, 2018 Edition: free - free allocated memory.

[geoffw0]

Changes LGTM. A sample of the new DCA results LGTM.

We could replace FreeCall with DeallocationExpr and fc.getArgument(0) with fc.getFreedExpr(), to add results for all kinds of free variants including, I think, delete. This is assuming we're confident all of these variants are in fact safe on NULL. It probably makes sense to promote the query as it is and consider this as a follow-up step afterwards?

Needs a docs review.

[jketema]

It probably makes sense to promote the query as it is and consider this as a follow-up step afterwards?

That would have my preference.

[mchammer01]

I'll review this on behalf of Docs.

[mchammer01]

LGTM ✨
Approving this but left a few comments and suggestions following editorial review.

[jketema]

LGTM ✨ Approving this but left a few comments and suggestions following editorial review.

Thanks for the review and the suggestions!

[jketema]

@geoffw0 Would you mind re-approving so we can merge this?

[geoffw0]

👍