Szelethus added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:109-110
bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
+ if (!Call.isGlobalCFunction())
+ return false;
+
----------------
balazske wrote:
> Szelethus wrote:
> > Isn't this redundant with my other inline about parameter types?
> Probably change to `isInSystemHeader` or use both?
Actually, this looks fine. How about preserving this...
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:127-131
+ for (auto P : Call.parameters()) {
+ QualType T = P->getType();
+ if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
+ return nullptr;
+ }
----------------
Szelethus wrote:
> balazske wrote:
> > Szelethus wrote:
> > > I'm not sure why we need this, is it true that *all* stream related
> > > functions return a pointer or a numerical value? Are we actually checking
> > > whether this really is a library function? If so, this looks pretty
> > > arbitrary.
> > This comes from code of CStringChecker:
> > ```
> > // Pro-actively check that argument types are safe to do arithmetic upon.
> > // We do not want to crash if someone accidentally passes a structure
> > // into, say, a C++ overload of any of these functions. We could not check
> > // that for std::copy because they may have arguments of other types.
> > ```
> > Still I am not sure that the checker works correct with code that contains
> > similar named but "arbitrary" functions.
> Oops, meant to write that ", is it true that *all* stream related functions
> have only pointer or a numerical parameters?".
...and removing this one, or changing it to an assert?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:156
+void StreamChecker::evalFread(const CallEvent &Call, CheckerContext &C) const {
+ (void)CheckNullStream(Call.getArgSVal(3), C);
}
----------------
balazske wrote:
> Szelethus wrote:
> > Why the need for `(void)`? `CheckNullSteam` doesn't seem to have an
> > `LLVM_NODISCARD` attribute.
> I wanted to be sure to get no buildbot compile errors (where -Werror is used).
They actually break on this?? Let me guess, is it the windows one? :D
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67706/new/
https://reviews.llvm.org/D67706
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
