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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
