[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

D126801 <https://reviews.llvm.org/D126801> should simplify the reinterpret casts.



================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:558
+                  InAlpha,
+                  Hide>, // ??
+  ]>,
----------------
I think we have 'Hidden', but not 'Hide'.
It means that it is a modeling checker. Any checker that emits diagnostics, should **not** be marked 'Hidden'.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:267-268
+  if (const auto *VD = dyn_cast_or_null<VarDecl>(D))
+    return VD->getIdentifier() &&
+           VD->getIdentifier()->getName() == ErrnoVarName;
+  if (const auto *FD = dyn_cast_or_null<FunctionDecl>(D))
----------------
It looks odd that in the very next `if` block you have `II` and here you don't.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h:34
+  /// value.
+  Errno_MustNotBeChecked = 2
+};
----------------
You don't need to prefix these with `Errno_`. It's already contained by a specific namespace.
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
> Unless the enumerators are defined in their own small namespace or inside a class, enumerators should have a prefix corresponding to the enum declaration name


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122150