[PATCH] D45059: Add a clang-tidy check to catch comparisons in TEMP_FAILURE_RETRY

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv created this revision.
george.burgess.iv added reviewers: aaron.ballman, alexfh.
Herald added a subscriber: mgorny.

`TEMP_FAILURE_RETRY(x)` is a macro that executes `x` until `(x) != -1 || errno != EINTR`, and evaluates to the result of the last execution of `x`.

I've been told that a somewhat common mistake people make with this is to include a condition in `TEMP_FAILURE_RETRY`'s argument, e.g.

  // Wrote
  TEMP_FAILURE_RETRY(foo() == 1)
  // Meant
  TEMP_FAILURE_RETRY(foo()) == 1

...In the former case, the `TEMP_FAILURE_RETRY` is useless, since `(x == 1) != -1` is always true. It will appear to work, though, since `TEMP_FAILURE_RETRY` will execute `foo()` at least once, and it'll evaluate to the value of the condition.

We do have a warning in clang to catch things like `(x == y) == -1` (-Wtautological-constant-out-of-range-compare), and it does complain about this in C++. However, we don't see these warnings in C, since `TEMP_FAILURE_RETRY` stores the result of executing `x` into a temporary of type `typeof(x)`, and comparisons are `int`s in C.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45059

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ComparisonInTempFailureRetryCheck.cpp
  clang-tidy/bugprone/ComparisonInTempFailureRetryCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-comparison-in-temp-failure-retry.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-comparison-in-temp-failure-retry.c

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45059.140312.patch
Type: text/x-patch
Size: 14046 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180329/8bdb4893/attachment.bin>