[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

[Balázs Kéri via Phabricator via cfe-commits]

balazske marked 2 inline comments as done.
balazske added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:396
+  /// yes was the value obtained or not.
+  mutable Optional<Optional<int>> KernelZeroSizePtrValue;
+
----------------
martong wrote:
> Which one is referred to the lazy initialization? The inner or the outer?
> These questions actually made me to come up with a more explanatory construct here:
> Could we do something like this?
> ```
> using LazyInitialized = Optional<int>;
> mutable Optional<LazyInitialized> KernelZeroSizePtrValue; // Or Lazy<Optional<...>>
> ```
Probably use a `std::unique_ptr<Optional<int>>` instead (like at the bug types, they could be optional too)?
If there is a code like
```
bool IsSomethingInitialized;
int Something;
```
that looks as a clear case to use an optional (or unique_ptr)? And if yes, is a reason for not using this construct if `int` is replaced by `Optional<int>`?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1687
+  if (ArgValKnown) {
+    if (!KernelZeroSizePtrValue)
+      KernelZeroSizePtrValue =
----------------
martong wrote:
> martong wrote:
> > This is a bit confusing for me. Perhaps alternatively we could have a free function `isInitialized(KernelZero...)` instead. Or maybe having a separate bool variable to indicate whether it was initialized could be cleaner?
> Another idea: Adding a helper struct to contain the bool `initialized`? E.g. (draft):
> ```
> struct LazyOptional {
>   bool initialized = false;
>   Opt<int> value;
>   Opt& get();
>   void set(const Opt&);
> };
> ```
It may be OK to have a function `lazyInitKernelZeroSizePtrValue`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76830/new/

https://reviews.llvm.org/D76830