balazske marked 5 inline comments as done. balazske added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:37-40 + // NoError: No error flag is set or stream is not open. + // EofError: EOF condition (feof returns true) + // OtherError: other (non-EOF) error (ferror returns true) + // AnyError: EofError or OtherError ---------------- Szelethus wrote: > These too. Also, I'm not yet sure whether we need `OtherError` and > `AnyError`, as stated in a later inline. I plan to use `AnyError` for failing functions that can have **EOF** and other error as result. At a later `ferror` or `feof` call a new state split follows to differentiate the error (instead of having to add 3 new states after the function, for success, EOF error and other error). If other operation is called we know anyway that some error happened. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:333-335 + // Ignore the call if the stream is is not tracked. + if (!State->get<StreamMap>(StreamSym)) + return; ---------------- balazske wrote: > Szelethus wrote: > > If we check in `preCall` whether the stream is opened why don't we > > conservatively assume it to be open? > If we do that we get a resource leak error for example in the test function > `pr8081` (there is only a call to `fseek`). We can assume that if the > function gets the file pointer as argument it does not "own" it so the close > is not needed. Otherwise the false positive resource leaks come always in > functions that take a file argument and use file operations. Or this case > (the file pointer is passed as input argument, the file is not opened by the > function that is analyzed) can be handled specially, we can assume that there > is no error initially and closing the file is not needed. This can be done in a next change. It involves more changes at other places. I think of inserting the state for the stream if it was not there before. But we need to save if this was such an insert or a normal `fopen` (and do not report resource leak for the "insert" case). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75356/new/ https://reviews.llvm.org/D75356 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
