balazske marked an inline comment as done.
balazske added a comment.
So, I think a different approach can be to store the stream state for the
stream instead of `ShouldCallX` variables. (The state can be no-error,
eof-error, other-error.) Optionally a warning can be made if any (modeled)
stream function is called and the stream is in error state (and handle feof and
ferror specially). (This is optional because it is not necessary to check the
error always, it should not cause crash or undefined behavior. If a function is
called in already failed stream state, it should simply fail again, or even
not. Retrying a read for example can be correct. To detect if there is any
error checking in the code the "ErrorReturnChecker" in
https://reviews.llvm.org/D72705 is the better solution.) A special case is if
the last called function was `getc` and the stream is in error state, here we
know that it is necessary to call `ferror` or `feof`.
================
Comment at: clang/test/Analysis/stream.c:246-247
+ if (C == EOF) {
+ feof(fp);
+ ferror(fp);
+ }
----------------
Szelethus wrote:
> balazske wrote:
> > Szelethus wrote:
> > > I bet this isn't how we envision how these function to be used. Maybe
> > > `ReturnValueChecker` could be deployed here? In any case, that would be a
> > > topic of a different revision.
> > If `fgetc` returns non-EOF we can be sure that there is no error, so no
> > need to call `ferror` and `feof`. If it returns EOF we must "double-check"
> > that it was a real error or an EOF-looking character that was read.
> Yea, but shouldn't we check the return values of `ferror` and `feof`?
> Otherwise whats the point?
Yes but this is job of an other checker (return value not used). This here is
to ensure that the functions are called at least.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75045/new/
https://reviews.llvm.org/D75045
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
