[PATCH] D140387: [clang][analyzer] Add stream related functions to StdLibraryFunctionsChecker.
Kristóf Umann via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 4 06:35:00 PST 2023
Szelethus added a comment.
LGTM, granted you add that test in the followup commit. If possible, I'd prefer to have features tested in the patch that added them (but this is fine for now).
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:838
+ Result += getArgDesc(ArgN);
+ Result += DK == Violation ? " should not be zero" : " is not zero";
+ return Result.c_str();
----------------
balazske wrote:
> Szelethus wrote:
> > I don't mean to make you test every single `Case` or `ArgumentConstraint` you added, but `NotZeroConstraint` is brand new, and is not tested anywhere.
> This new class is probably not needed at all. It is always possible to use `ReturnValueCondition(WithinRange, SingleValue(0))` or similar for nonzero cases.
Convenience is a good enough reason. Please test this before commiting.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1848-1849
+ // int fseek(FILE *stream, long offset, int whence);
+ // FIXME: It is possible to get the 'SEEK_' values (like EOFv) for arg 2
+ // condition.
+ addToFunctionSummaryMap(
----------------
balazske wrote:
> Szelethus wrote:
> > Sure, but what should be fixed? We should check whether the the last argument is a `SEEK_*` value?
> It could be an improvement to detect possible values for the argument 2 that are the `SEEK_*` values. Now the range [0,2] is used (the `SEEK_*` constants are usually 0,1,2).
Could you please more-or-less copy-paste this inline into the code?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140387/new/
https://reviews.llvm.org/D140387
More information about the cfe-commits
mailing list