whisperity added a comment.
I agree with @martong that `$ = realloc($, ...)` is indicative of something
going wrong, even if the developer has saved the original value somewhere. Are
we trying to catch this with Tidy specifically for this reason (instead of
CSA's stdlib checker, or some taint-related mechanism?). However, @steakhal has
some merit in saying that developers might prioritise the original over the
copy, even if they made a copy. I think an easy solution from a documentation
perspective is to have a test for this at the bottom of the test file. And
document that we cannot (for one reason or the other) catch this supposed
solution, **but** //if// you have made a copy already(!), then you might as
well could have done `void* q = realloc(p, ...)` anyway! Having this documented
leaves at least //some// paths open for us to fix false positives later, if
they become a tremendous problem. (I have a gut feeling that they will not,
that much.)
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.cpp:20-21
+namespace {
+class IsSamePtrExpr : public clang::StmtVisitor<IsSamePtrExpr, bool> /*,
+ public clang::DeclVisitor<IsSamePtrExpr, bool>*/
+{
----------------
steakhal wrote:
> Commented code?
(Nit: `using namespace clang` -> The `clang::` should be redundant here.)
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.cpp:87
+ return;
+ if (!IsSamePtrExpr().check(PtrInputExpr, PtrResultExpr))
+ return;
----------------
(We're creating a temporary here, right?)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133119/new/
https://reviews.llvm.org/D133119
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
