[PATCH] D79437: [clang-tidy] Add fsetpos argument checker

Beka Grdzelishvili via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun May 24 11:44:28 PDT 2020


DerWaschbar added a comment.

In D79437#2052109 <https://reviews.llvm.org/D79437#2052109>, @aaron.ballman wrote:

> Thank you for working on this check, I think it's useful functionality. One concern I have, though, is that it's not flow-sensitive and should probably be implemented as a clang static analyzer check instead of a clang-tidy check. For instance, consider these three plausible issues:
>
>   // This only sets the offset on one code path.
>   void func(FILE *fp) {
>     fpos_t offset;
>     if (condition) {
>       // ... code
>       if (0 != fgetpos(fp, &offset))
>         return;
>       // ... code
>     } else {
>      // ... code
>     }
>     fsetpos(fp, &offset);
>   }
>  
>   // This doesn't check the failure from getting the position and sets the position regardless.
>   void func(FILE *fp) {
>     fpos_t offset;
>     fgetpos(fp, &offset);
>     // ... code
>     fsetpos(fp, &offset);
>   }
>  
>   // This function accepts the offset from the caller but the caller passes an invalid offset.
>   void func(FILE *fp, const fpos_t *offset) {
>     fsetpos(fp, offset);
>   }
>   void caller(FILE *fp) {
>     fpos_t offset;
>     func(fp, &offset);
>   }
>
>
> Have you considered writing a static analyzer check so you can do data and control flow analysis to catch issues like these?


I have noticed those issues too, but most likely the getter/setter will be in the same function body and we could measure fast how common is that issue in the wild. Also, this was my first introductory project for Clang and with that, I can rewrite this as a Static Analyzer project or start working on another Clang-Tidy project.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D79437





More information about the cfe-commits mailing list