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

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D75851#1914874 <https://reviews.llvm.org/D75851#1914874>, @balazske wrote:

> I do not like to make difference between error from `fseek` and error from any other function that may fail. `AnyError` can be used to handle every case.


Okay, it took me a while, but I think I understand where you're coming from. Lets preserve this enum. I think `NoteTags` would be a better fit to pinpoint which function left the stream in a state that should be checked before usage. This patch is quite small, could we add one then?

> After a failed file operation we do not know if it was feof or ferror kind of error.

Is this true for that many stream operations?



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:382-384
+  C.addTransition(StateNotFailed);
+  C.addTransition(StateFailedWithFError);
+  C.addTransition(StateFailedWithoutFError);
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75851/new/

https://reviews.llvm.org/D75851