steakhal added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:286-287
+                      SVal V,
+                      std::function<bool(RegionAndSymbolInvalidationTraits &,
+                                         const MemRegion *)>);
 
----------------
I'd highly suggest making this a template taking the functor that way.
Given that we modify these functions, we should make them comply with the lower 
camel case naming convention.
And their variables with upper camel case.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1054
+    SVal SizeV, QualType SizeTy) {
+  return InvalidateBufferAux(
+      C, S, BufE, BufV,
----------------
Shouldn't we assert that `SizeV` is some known value or at least smaller than 
(or equal to) the extent of the buffer?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:1075-1077
+      [](RegionAndSymbolInvalidationTraits &, const MemRegion *R) {
+        return MemRegion::FieldRegionKind == R->getKind();
+      });
----------------
The lambdas inline in the call look a bit messy. I would prefer declaring it 
and passing it as an argument separately.
This applies to the rest of the lambdas as well.


================
Comment at: clang/test/Analysis/issue-55019.cpp:13-14
+
+void *malloc(size_t);
+void free(void *);
+
----------------
OikawaKirie wrote:
> > Ah, I see that it's for c function declarations. If that's the case, have 
> > you considered adding the malloc and free declarations to that header?
> 
> What about doing this in another patch and updating all test cases that use 
> malloc and free? Maybe other libc APIs as well?
> The test case malloc-three-arg.c declares malloc and in a different signature 
> and also includes this header. Simply doing so in this patch will lead to 
> other conflicts.
Okay.


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

https://reviews.llvm.org/D152435

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to