aaron.ballman added inline comments.
================
Comment at: clang-tidy/bugprone/IoFunctionsCheck.cpp:47-49
+ "consider to cast the return value of %0 from type integer to type char,
"
+ "possible loss of precision if an error has occurred or the end "
+ "of file has been reached");
----------------
hgabii wrote:
> aaron.ballman wrote:
> > This diagnostic confuses me, so perhaps you can explain the situation you
> > want to diagnose a bit further.
> >
> > FIO34-C is about situations where `sizeof(char) == sizeof(int)` and the
> > call returns EOF/WEOF. In that case, there's no way to distinguish between
> > an EOF/error and a valid character. Suggesting to explicitly cast to a
> > character type doesn't add any safety to the code -- the user needs to
> > insert calls to `feof()` or `ferror()` instead (to make it portable,
> > anyway) and should store the character value in a sufficiently large
> > integer type before doing the comparison against EOF.
> >
> > Are you trying to catch a different kind of bug?
> Yes, I want to diagnose this problem. Do you think I need to add suggestions
> into the warning message to use `feof()` or `ferror()`?
I think there are two different situations to think about:
1) If the target architecture has `sizeof(char) == sizeof(int)`, no amount of
casting will help the user because the bit pattern for `EOF` is the same
whether stored as an `int` or a `char` on that platform. There, the user really
must use `feof()` and `ferror()`. This situation is theoretically even worse
for the wide versions of the APIs because `sizeof(wint_t) == sizeof(wchar_t)`
quite commonly, except that it would require a terrible choice in character
sets for `wchar_t` by the implementation because Unicode makes guarantees about
certain bit patterns not being a valid character (like 0xFFFF in UTF-16). I
don't know that we have to worry about the wide-char versions at all in
practice, but if you know of a platform where this is a legitimate concern, I'd
love to hear about it.
2) If the target architecture does not have this
identical-size-in-band-error-indicator problem, then it might make sense to
diagnose the code for portability reasons or it might make sense to not
diagnose at all because the user won't run into it. Might be worth making this
an optional feature of the check.
Either way, I don't know that we'll be able to suggest fixes for the user in a
diagnostic message of reasonable length. This might be a case where we want to
tell the user about the danger and let the documentation tell them how to fix
it. Maybe a diagnostic like: `"the value for %select{EOF|WEOF}0 is
indistinguishable from a valid character with the same bit pattern; consider
explicit checks for error or end-of-file indicators"`
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D42682/new/
https://reviews.llvm.org/D42682
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
