Szelethus added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:478 + + // FIXME: Check for stream in EOF state? + ---------------- balazske wrote: > Szelethus wrote: > > Will that be the next patch? :D Eager to see it! > It may be too big change to add it as well here (or cause more difficulties). > But it should be a check for `ErrorFEof` in the `ErrorState` (and add > another bug type). This is not a fatal error. > > It may be more difficult to make `fread` return the feof error again if it > starts already with feof state. (Because the state with FEof should be split > from the generic error state that can contain other possible errors.) > It may be too big change to add it as well here (or cause more difficulties). > But it should be a check for ErrorFEof in the ErrorState (and add another bug > type). This is not a fatal error. Oh, yea, right, we went over this once :) What I really meant, is an EOF related error, like reading a variable with an EOF value in any context that isn't a comparison to the actual `EOF` (which in many contexts still isn't a //fatal// error, but is far more an indicator of code smell). But I'm just thinking aloud, please proceed with this project however it is convenient to you! > It may be more difficult to make fread return the feof error again if it > starts already with feof state. (Because the state with FEof should be split > from the generic error state that can contain other possible errors.) I gave this plenty of thought, how do you imagine the problem of us not splitting up to 3 states to show up? Suppose we're calling fread on a stream where the error state is either EOF or ERROR, but we don't know which. We could just leave the `StreamErrorState` as-is, couldn't we? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78374/new/ https://reviews.llvm.org/D78374 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
