balazske marked 2 inline comments as done.
balazske added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:394
+ StreamSym, StreamState::getOpenedWithOtherError());
+ C.addTransition(StateEof);
+ C.addTransition(StateOther);
----------------
baloghadamsoftware wrote:
> xazax.hun wrote:
> > As you probably know we are splitting the execution paths here. This will
> > potentially result in doubling the analysis tome for a function for each
> > `feof` call. Since state splits can be really bad for performance we need
> > good justification for each of them.
> > So the questions are:
> > * Is it really important to differentiate between eof and other error or is
> > it possible to just have an any error state?
> > * Do we find more bugs in real world code that justifies these state splits?
> I agree here. Do not introduce such forks without specific reason.
`feof` and `ferror` should return the same value if called multiple times (and
no other file operations in between). If the exact error is not saved in the
state, how can we ensure that the calls return the same value?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:404-405
+
+void StreamChecker::evalFerror(const FnDescription *Desc, const CallEvent
&Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
----------------
baloghadamsoftware wrote:
> balazske wrote:
> > Szelethus wrote:
> > > This function is practically the same as `evalFeof`, can we merge them?
> > It is not the same code ("EOF" and "other error" are interchanged), but can
> > be merged somehow.
> Maybe using a template with a functor parameter and pass a lambda in the two
> instantiations? (See `DebugContainerModeling` and `DebugIteratorModeling`)
> and also some example is `Iterator.cpp`.
I have already a template implementation, but probably it is not needed now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75682/new/
https://reviews.llvm.org/D75682
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
