[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

[Whisperity via Phabricator via cfe-commits]

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