[PATCH] D80015: [Analyzer][StreamChecker] Added support for 'fread' and 'fwrite'.

Mon May 18 09:07:38 PDT 2020

Szelethus added a comment.

In D80015#2041653 <https://reviews.llvm.org/D80015#2041653>, @balazske wrote:

> The difference of fread and fwrite comes from the fact that `fwrite` can always succeed, `fread` does not succeed in EOF state.
>  The plan is to add the file functions sequentially. From the currently implemented functions only `fread` needs the warning in EOF state. When any later function is added that fails in EOF state, the warning should be added for that function at that time.

Ah, silly me. Like, obviously, where would you like write the file, if not at the end of it...

In any case, the warning would be a better fit as a standalone revision because FERROR also deserves a similar warning I think, not to mention that plenty of other stream operations should check against this.

================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:495
+    return;
+  Optional<NonLoc> CountVal = Call.getArgSVal(2).getAs<NonLoc>();
+  if (!CountVal)
----------------
balazske wrote:
> Szelethus wrote:
> > `// The standard refers to this parameter as "nmemb", but almost everywhere else its called "count".`
> I looked up the functions in [[ https://en.cppreference.com/w/c/io/fread | cppreference.com ]], there it is called `count`. But probably it is better to use the standard name here?
Its not a big deal, either a comment or a variable name change should be fine. I'll leave it to your judgement, but its not a bad idea to leave a line about the most commonly referred name ("count") is not the same as the standard one ("nmemb").

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80015