jcai19 marked 10 inline comments as done.
jcai19 added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/android/PosixReturnCheck.cpp:23
+ binaryOperator(
+ hasOperatorName("<"),
+ hasLHS(callExpr(callee(functionDecl(matchesName("^::posix_"),
unless(hasName("posix_openpt")))))),
----------------
george.burgess.iv wrote:
> should we also try to catch `<= ${negative_value}` (or `< ${negative_value}`)?
Sounds good. Will update the patch to catch these two cases as well.
================
Comment at: clang-tools-extra/clang-tidy/android/PosixReturnCheck.cpp:29
+ binaryOperator(
+ hasOperatorName("=="),
+ hasLHS(callExpr(callee(functionDecl(matchesName("^::posix_"),
unless(hasName("posix_openpt")))))),
----------------
george.burgess.iv wrote:
> similarly, should we complain about `!= ${negative_value}`?
Sounds good. Will update the patch to catch these additional cases as well.
================
Comment at: clang-tools-extra/clang-tidy/android/PosixReturnCheck.cpp:39
+ const auto &BinOp = *Result.Nodes.getNodeAs<BinaryOperator>("binop");
+ diag(BinOp.getOperatorLoc(), "posix functions (except posix_openpt) never
return negative values");
+}
----------------
george.burgess.iv wrote:
> would it be helpful to add fixits for simple cases? e.g. we can probably
> offer to replace `posix_whatever() < 0` with `posix_whatever() > 0` or `!= 0`
While this fix is handy, I am not sure whether it will be safe enough under all
circumstances. For example, is it possible in the code block following the
check, the program calls another POSIX function and alter the errno before its
value it checked? In that case, maybe the proper fix should be something as
follows and fixing it by changing the binary operator may obscure it:
int ret = posix_whatever();
if (ret != 0)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63623/new/
https://reviews.llvm.org/D63623
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
