baloghadamsoftware added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:382-384 + C.addTransition(StateNotFailed); + C.addTransition(StateFailedWithFError); + C.addTransition(StateFailedWithoutFError); ---------------- Szelethus wrote: > balazske wrote: > > Szelethus wrote: > > > This seems a bit excessive, we could merge the last two into `FSeekError`. > > There are 3 cases: > > - `fseek` did not fail at all. Return value is zero. This is > > `StateNotFailed`. > > - `fseek` failed but none of the error flags is true afterwards. Return > > value is nonzero but `feof` and `ferror` are not true. This is > > `StateFailedWithoutFError`. > > - `fseek` failed and we have `feof` or `ferror` set (never both). Return > > value is nonzero and `feof` or `ferror` will be true. This is > > `StateFailedWithFError`. And an use of `AnyError`, otherwise we need here 2 > > states, one for `feof` and one for `ferror`. But it is not important here > > if `feof` or `ferror` is actually true, so the special value `AnyError` is > > used and only one new state instead of two. > Sure, but from the point of the analyzer the latter two are the same, isn't > it? Its never a good idea to use a stream on which `fseek` failed without > checking. > > State splits are the most expensive things the analyzer can make, which is > why I'm cautious here. Somehow, this should be done with just two new states. Maybe there should be an error state "Unknown" instead of `OtherError` (or `FeoFOrFError` what I suggested at the other patch) which can be any of the errors plus the `NoError` value. Later, when `ferror()` of `feof()` is checked we can do a second state split, but not earlier. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75851/new/ https://reviews.llvm.org/D75851 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
