[PATCH] D69948: [Checkers] Added support for freopen to StreamChecker.

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Thank you for working on this. Please add comments to the code and maybe also to the tests. I could not find anything in the standards about `freopen()` with null-pointer for the stream parameter.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:176
+  const LocationContext *LCtx = C.getPredecessor()->getLocationContext();
+  ConstraintManager &CM = C.getConstraintManager();
+  auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
----------------
The four lines above are typical cases for using auto, since the type is duplicated in the line. See: https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:220
+    StateRetNull = CM.assume(StateRetNull, RetVal, false);
+    assert(StateRetNull && "This assumption should not fail.");
+    StateRetNull =
----------------
Please use a more elaborated error message here.


================
Comment at: clang/test/Analysis/stream.c:120
+  fclose(p2); // expected-warning {{Try to close a file Descriptor already closed. Cause undefined behaviour}}
+}
+
----------------
I wonder if this test belongs into this particular patch. It has nothing to do with `freopen()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69948