whisperity added inline comments.

================
Comment at: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.cpp:89-90
+    return;
+  diag(Call->getBeginLoc(),
+       "memory block passed to 'realloc' may leak if 'realloc' fails");
+}
----------------
I have some reservations about this message, though. It only indirectly states 
the problem: first, the user needs to know what a memory leak is, and then 
needs to know how realloc could fail, and then make the mental connection about 
the overwriting of the pointer. (Also, you're passing a pointer... "memory 
block" is a more abstract term, requiring more knowledge.)

I think for the sake of clarity, we should be more explicit about this.

> taking the returned pointer from 'realloc' overwrites 'XXXPtr' with null
> if allocation fails,
> which may result in a leak of the original buffer

1st line explains how the issue manifests (explicitly saying that you'll get a 
nullpointer in a bad place)
2nd line gives the condition, which is a bit more explicit
3rd line explains the real underlying issue

(Getting the null pointer is the "only" bad path here.)


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

Reply via email to