[PATCH] D75851: [Analyzer][StreamChecker] Added evaluation of fseek.

[Balogh, Ádám via Phabricator via cfe-commits]

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