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

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

balazske added a comment.

If the unit of the change is adding `fread` and `fwrite` completely, the warning with FEOF at `fread` is correct to add because it belongs to `fread`. I was planning to add some file handling functions to the checker that have a basic functionality implemented. In this case, it is the fread-fwrite functions, but the "indeterminate file position" is still missing here (that is a bigger change), so it is the "basic functionality" that is added. The FEOF warning is probably too small to add it in a separate patch, also it is added only to one place now.

There is intentionally no warning about ferror state, first reason is that often ferror and "indeterminate file position" comes together and then the warning will be produced for "indeterminate position" in the next change. Next reason is that ferror in itself is not always cause for warning and we can currently not decide about it. There was the example about write into a file that is open in read mode that results in a ferror state, then the next read should work (with ferror state at start). Another reason, the program should still always handle if a file operation fails. If an operation starts in ferror state, it will probably fail again but that error should be handled by the application.

Intent of these warnings is not to check that the error case was handled by the user code, that is a more difficult thing. We could make a new warning that warns if between two possible failing operations no `ferror` or `feof` was called, this would check if the user handled the error. But not accurate because the error can be handled based on the return value of the failed function, without calling `ferror` or `feof` (except some special cases like the `fgetc`). The only safe warning is to add if a function is called in really unuseful way, for example fread in already FEOF state that do never succeed, and the "indeterminate file position" case.

My goal with these changes was to have a check for the CERT FIO34-C case, not for the complete `StreamChecker` that could contain many other features. So probably not even all file handling functions will be added for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80015