[PATCH] D78374: [Analyzer][StreamChecker] Added evaluation of fread and fwrite.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:152-158
 /// Get the value of the stream argument out of the passed call event.
 /// The call should contain a function that is described by Desc.
 SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
   assert(Desc && Desc->StreamArgNo != ArgNone &&
          "Try to get a non-existing stream argument.");
   return Call.getArgSVal(Desc->StreamArgNo);
 }
----------------
Szelethus wrote:
> If we add methods to `FnDescription`, we might as well add this as well, but I don't insist.
Now this is again the only function that could be member, in the current form the assert can be done but not in the member form.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:37
+/// Note: A stream has either FEOF or FERROR set but not both at the same time.
+struct StreamErrorState {
+  /// There is an execution path with none of the error flags set.
----------------
Szelethus wrote:
> We're not describing the error state of a stream here, but rather //possible// error states, so the name should reflect that.
I do not think that `StreamPossibleErrorState` is better, it is anyway a //state// that can contain any information. It is an error related state even if it defines not one exact error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78374