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