Update clang-tidy

Fix a bunch of minor issues, ignore others.  We use a lot of global
variables.  Enabling a check will hopefully make sure we don't add more.

Change-Id: Ie76053d4afc95a5372b70b2fb768be464468cec0
Signed-off-by: Ed Tanous <[email protected]>

Author: edtanous

.clang-tidy

@@ -3,17 +3,24 @@ Checks: '
 boost-use-to-string,
 bugprone-argument-comment,
 bugprone-assert-side-effect,
+bugprone-assignment-in-if-condition,
 bugprone-bad-signal-to-kill-thread,
 bugprone-bool-pointer-implicit-conversion,
 bugprone-branch-clone,
+bugprone-casting-through-void,
+bugprone-chained-comparison,
+bugprone-compare-pointer-to-member-virtual-function,
 bugprone-copy-constructor-init,
 bugprone-dangling-handle,
 bugprone-dynamic-static-initializers,
+bugprone-empty-catch,
 bugprone-fold-init-type,
 bugprone-forward-declaration-namespace,
 bugprone-forwarding-reference-overload,
 bugprone-implicit-widening-of-multiplication-result,
 bugprone-inaccurate-erase,
+bugprone-inc-dec-in-conditions,
+bugprone-incorrect-enable-if,
 bugprone-incorrect-roundings,
 bugprone-infinite-loop,
 bugprone-integer-division,
@@ -24,35 +31,49 @@ bugprone-misplaced-operator-in-strlen-in-alloc,
 bugprone-misplaced-pointer-arithmetic-in-alloc,
 bugprone-misplaced-widening-cast,
 bugprone-move-forwarding-reference,
+bugprone-multi-level-implicit-pointer-conversion,
+bugprone-multiple-new-in-one-expression,
 bugprone-multiple-statement-macro,
 bugprone-no-escape,
+bugprone-non-zero-enum-to-bool-conversion,
 bugprone-not-null-terminated-result,
+bugprone-optional-value-conversion,
 bugprone-parent-virtual-call,
 bugprone-posix-return,
 bugprone-redundant-branch-condition,
 bugprone-reserved-identifier,
+bugprone-shared-ptr-array-mismatch,
 bugprone-signal-handler,
 bugprone-signed-char-misuse,
 bugprone-sizeof-container,
 bugprone-sizeof-expression,
 bugprone-spuriously-wake-up-functions,
+bugprone-standalone-empty,
 bugprone-string-constructor,
 bugprone-string-integer-assignment,
 bugprone-string-literal-with-embedded-nul,
+bugprone-stringview-nullptr,
 bugprone-suspicious-enum-usage,
 bugprone-suspicious-include,
+bugprone-suspicious-memory-comparison,
 bugprone-suspicious-memset-usage,
 bugprone-suspicious-missing-comma,
+bugprone-suspicious-realloc-usage,
 bugprone-suspicious-semicolon,
 bugprone-suspicious-string-compare,
 bugprone-swapped-arguments,
+bugprone-switch-missing-default-case,
 bugprone-terminating-continue,
 bugprone-throw-keyword-missing,
 bugprone-too-small-loop-variable,
+bugprone-unchecked-optional-access,
 bugprone-undefined-memory-manipulation,
 bugprone-undelegated-constructor,
 bugprone-unhandled-exception-at-new,
 bugprone-unhandled-self-assignment,
+bugprone-unique-ptr-array-mismatch,
+bugprone-unsafe-functions,
+bugprone-unused-local-non-trivial-variable,
 bugprone-unused-raii,
 bugprone-unused-return-value,
 bugprone-use-after-move,
@@ -89,8 +110,10 @@ cert-pos44-c,
 cert-pos47-c,
 cert-sig30-c,
 cert-str34-c,
+clang-analyzer-apiModeling.Errno,
 clang-analyzer-apiModeling.StdCLibraryFunctions,
 clang-analyzer-apiModeling.TrustNonnull,
+clang-analyzer-apiModeling.TrustReturnsNonnull,
 clang-analyzer-apiModeling.google.GTest,
 clang-analyzer-apiModeling.llvm.CastValue,
 clang-analyzer-apiModeling.llvm.ReturnValue,
@@ -111,6 +134,7 @@ clang-analyzer-core.uninitialized.ArraySubscript,
 clang-analyzer-core.uninitialized.Assign,
 clang-analyzer-core.uninitialized.Branch,
 clang-analyzer-core.uninitialized.CapturedBlockVariable,
+clang-analyzer-core.uninitialized.NewArraySize,
 clang-analyzer-core.uninitialized.UndefReturn,
 clang-analyzer-cplusplus.InnerPointer,
 clang-analyzer-cplusplus.Move,
@@ -120,6 +144,7 @@ clang-analyzer-cplusplus.PlacementNew,
 clang-analyzer-cplusplus.PureVirtualCall,
 clang-analyzer-cplusplus.SelfAssignment,
 clang-analyzer-cplusplus.SmartPtrModeling,
+clang-analyzer-cplusplus.StringChecker,
 clang-analyzer-cplusplus.VirtualCallModeling,
 clang-analyzer-deadcode.DeadStores,
 clang-analyzer-fuchsia.HandleChecker,
@@ -199,8 +224,10 @@ clang-analyzer-valist.Unterminated,
 clang-analyzer-valist.ValistBase,
 clang-analyzer-webkit.NoUncountedMemberChecker,
 clang-analyzer-webkit.RefCntblBaseVirtualDtor,
+clang-analyzer-webkit.UncountedLambdaCapturesChecker,
 cppcoreguidelines-avoid-c-arrays,
 cppcoreguidelines-avoid-goto,
+cppcoreguidelines-avoid-non-const-global-variables,
 cppcoreguidelines-c-copy-assignment-signature,
 cppcoreguidelines-explicit-virtual-functions,
 cppcoreguidelines-init-variables,
@@ -218,12 +245,20 @@ cppcoreguidelines-pro-type-static-cast-downcast,
 cppcoreguidelines-pro-type-union-access,
 cppcoreguidelines-slicing,
 google-explicit-constructor,
+misc-coroutine-hostile-raii,
+misc-header-include-cycle,
+misc-misleading-bidirectional,
+misc-misleading-identifier,
 misc-misplaced-const,
+misc-new-delete-overloads,
+misc-non-copyable-objects,
 misc-redundant-expression,
 misc-static-assert,
 misc-throw-by-value-catch-by-reference,
 misc-unconventional-assign-operator,
 misc-uniqueptr-reset-release,
+misc-unused-alias-decls,
+misc-unused-parameters,
 misc-unused-using-decls,
 modernize-avoid-bind,
 modernize-deprecated-headers,
@@ -265,8 +300,11 @@ performance-type-promotion-in-math-fn,
 performance-unnecessary-copy-initialization,
 performance-unnecessary-value-param,
 readability-avoid-const-params-in-decls,
+readability-avoid-nested-conditional-operator,
 readability-braces-around-statements,
 readability-const-return-type,
+readability-container-contains,
+readability-container-data-pointer,
 readability-container-size-empty,
 readability-convert-member-functions-to-static,
 readability-delete-null-pointer,
@@ -287,6 +325,7 @@ readability-redundant-access-specifiers,
 readability-redundant-control-flow,
 readability-redundant-declaration,
 readability-redundant-function-ptr-dereference,
+readability-redundant-inline-specifier,
 readability-redundant-member-init,
 readability-redundant-preprocessor,
 readability-redundant-smartptr-get,
@@ -300,6 +339,7 @@ readability-string-compare,
 readability-suspicious-call-argument,
 readability-uniqueptr-delete-release,
 readability-uppercase-literal-suffix'
+
 WarningsAsErrors: '*'
 HeaderFilterRegex: '.*'
 CheckOptions:
@@ -309,3 +349,5 @@ CheckOptions:
   - { key: readability-identifier-naming.ParameterCase, value: camelBack }
   - { key: readability-identifier-naming.NamespaceCase, value: lower_case }
   - { key: readability-identifier-naming.StructCase,    value: CamelCase  }
+  - { key: cppcoreguidelines-rvalue-reference-param-not-moved.IgnoreUnnamedParams, value: true }
+  - { key: misc-include-cleaner.IgnoreHeaders, value: ((nlohmann/json_fwd.hpp)|(boost/.*/src.hpp)|(boost/.*/detail/.*)|(nlohmann/detail/.*)|(stdio.h)|(ranges)|(bits/chrono.h)|(boost/system/error_code.hpp)) }

src/entity_manager.cpp

@@ -68,6 +68,7 @@ using JsonVariantType =
                  int64_t, uint64_t, double, int32_t, uint32_t, int16_t,
                  uint16_t, uint8_t, bool>;
 
+// NOLINTBEGIN(cppcoreguidelines-avoid-non-const-global-variables)
 // store reference to all interfaces so we can destroy them later
 boost::container::flat_map<
     std::string, std::vector<std::weak_ptr<sdbusplus::asio::dbus_interface>>>
@@ -79,6 +80,7 @@ nlohmann::json lastJson;
 Topology topology;
 
 boost::asio::io_context io;
+// NOLINTEND(cppcoreguidelines-avoid-non-const-global-variables)
 
 const std::regex illegalDbusPathRegex("[^A-Za-z0-9_.]");
 const std::regex illegalDbusMemberRegex("[^A-Za-z0-9_]");

src/fru_device.cpp

@@ -72,6 +72,8 @@ const static constexpr char* baseboardFruLocation =
 
 const static constexpr char* i2CDevLocation = "/dev";
 
+// TODO Refactor these to not be globals
+// NOLINTBEGIN(cppcoreguidelines-avoid-non-const-global-variables)
 static boost::container::flat_map<size_t, std::optional<std::set<size_t>>>
     busBlocklist;
 struct FindDevicesWithCallback;
@@ -84,6 +86,7 @@ static boost::container::flat_map<size_t, std::set<size_t>> failedAddresses;
 static boost::container::flat_map<size_t, std::set<size_t>> fruAddresses;
 
 boost::asio::io_context io;
+// NOLINTEND(cppcoreguidelines-avoid-non-const-global-variables)
 
 bool updateFRUProperty(
     const std::string& updatePropertyReq, uint32_t bus, uint32_t address,
@@ -621,13 +624,13 @@ void loadBlocklist(const char* path)
                     auto addresses =
                         addressData.get<std::set<std::string_view>>();
 
-                    busBlocklist[bus].emplace();
+                    auto& block = busBlocklist[bus].emplace();
                     for (const auto& address : addresses)
                     {
                         size_t addressInt = 0;
                         std::from_chars(address.begin() + 2, address.end(),
                                         addressInt, 16);
-                        busBlocklist[bus]->insert(addressInt);
+                        block.insert(addressInt);
                     }
                 }
                 else
@@ -1416,6 +1419,7 @@ int main()
                 case IN_CREATE:
                 case IN_MOVED_TO:
                 case IN_DELETE:
+                {
                     std::string_view name(&iEvent->name[0], iEvent->len);
                     if (boost::starts_with(name, "i2c"))
                     {
@@ -1438,6 +1442,10 @@ int main()
                                      unknownBusObjectCount, powerIsOn,
                                      objServer, systemBus);
                     }
+                }
+                break;
+                default:
+                    break;
             }
             index += sizeof(inotify_event) + iEvent->len;
         }

src/fru_utils.cpp

@@ -191,6 +191,11 @@ std::pair<DecodeState, std::string>
             }
         }
         break;
+
+        default:
+        {
+            return make_pair(DecodeState::err, value);
+        }
     }
 
     return make_pair(DecodeState::ok, value);

src/fru_utils.hpp

@@ -38,6 +38,7 @@ constexpr size_t fruBlockSize = 8;
 using DeviceMap = boost::container::flat_map<int, std::vector<uint8_t>>;
 using BusMap = boost::container::flat_map<int, std::shared_ptr<DeviceMap>>;
 
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 inline BusMap busMap;
 
 enum class DecodeState

src/overlay.cpp

@@ -41,7 +41,7 @@ constexpr const char* muxSymlinkDir = "/dev/i2c-mux";
 
 constexpr const bool debug = false;
 
-std::regex illegalNameRegex("[^A-Za-z0-9_]");
+const std::regex illegalNameRegex("[^A-Za-z0-9_]");
 
 // helper function to make json types into string
 std::string jsonToString(const nlohmann::json& in)

src/perform_scan.cpp

@@ -24,11 +24,13 @@
 #include <charconv>
 
 /* Hacks from splitting entity_manager.cpp */
+// NOLINTBEGIN(cppcoreguidelines-avoid-non-const-global-variables)
 extern std::shared_ptr<sdbusplus::asio::connection> systemBus;
 extern nlohmann::json lastJson;
 extern void
     propertiesChangedCallback(nlohmann::json& systemConfiguration,
                               sdbusplus::asio::object_server& objServer);
+// NOLINTEND(cppcoreguidelines-avoid-non-const-global-variables)
 
 using GetSubTreeType = std::vector<
     std::pair<std::string,

src/utils.cpp

@@ -42,7 +42,9 @@
 constexpr const char* templateChar = "$";
 
 namespace fs = std::filesystem;
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 static bool powerStatusOn = false;
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 static std::unique_ptr<sdbusplus::bus::match_t> powerMatch = nullptr;
 
 bool findFiles(const fs::path& dirPath, const std::string& matchString,

src/utils.hpp

@@ -30,6 +30,7 @@ constexpr const char* configurationOutDir = "/var/configuration/";
 constexpr const char* versionHashFile = "/var/configuration/version";
 constexpr const char* versionFile = "/etc/os-release";
 
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 extern boost::asio::io_context io;
 
 using DBusValueVariant =