summaryrefslogtreecommitdiff
path: root/.clang-tidy
AgeCommit message (Collapse)AuthorFilesLines
2024-04-24Add new tidy checksEd Tanous1-1/+27
Another clang, another list of checks we can enable. Tested: Clang-tidy passes. Change-Id: Ib3fcd8046ce9f2caf9f9d95571ffa3cc2f56c596 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-19Add missing headersEd Tanous1-1/+1
Most of these were found by breaking every redfish class handler into its own compile unit: When that's done, these missing headers become compile errors. We should just fix them. In addition, this allows us to enable automatic header checking in clang-tidy using misc-header-cleaner. Because the compiler can now "see" all the defines, it no longer tries to remove headers that it thinks are unused. [1] https://github.com/openbmc/bmcweb/commit/4fdee9e39e9f03122ee16a6fb251a380681f56ac Tested: Code compiles. Change-Id: Ifa27ac4a512362b7ded7cc3068648dc4aea6ad7b Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Fix large copies with url_view and segments_viewEd Tanous1-1/+0
Despite these objects being called "view" they are still relatively large, as clang-tidy correctly flags, and we ignore. Change all function uses to capture by: const boost::urls::url_view_base& Which is the base class of all boost URL types, and any class (url, url_view, etc) is convertible to that base. Change-Id: I8ee2ea3f4cfba38331303a7e4eb520a2b6f8ba92 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-09Rework logger to create compile time errorsEd Tanous1-0/+1
The logger changes to move to std::format incidentally caused format errors to no longer be flagged at compile time. The example here[1] shows that the latest gcc/c++ gave us a way to solve this, using std::format_string. This incidentally shows two places where we got the format arguments wrong, so fix them. [1] https://stackoverflow.com/questions/72795189/how-can-i-wrap-stdformat-with-my-own-template-function Change-Id: Id884200e2c98eeaf5ef8db6c1d6362ede2ffb858 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-01Move where appropriateEd Tanous1-0/+1
Clang-tidy-18 has new checks that can find more cases where we've missed an opportunity to std::move. Fix them. Tested: Logging works, unit tests pass. Change-Id: I0cf58204ce7265828693b787a7b3a16484c3d5e5 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-01Enable readability checkEd Tanous1-0/+1
readability-avoid-nested-conditional-operator With one exception, we already pass this check. Update the log services code to make it pass, and update it to use the generated enums. Tested: Code inspection only. Change-Id: Ic80a7382beb0f541de4916d7b51e42ed5d5dc542 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-01Fix redundant inline operatorsEd Tanous1-0/+1
inline is not required on member methods. Clang-tidy has a check for this. Enable the check and fix the two bad usages. Tested: Code compiles. Change-Id: I3115b7c0c4005e1082e0005b818fbe6569511f49 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-28Add misc-include-cleanerEd Tanous1-0/+1
And fix the includes that are wrong. Note, there is a very large ignore list included in the .clang-tidy configcfile. These are things that clang-tidy doesn't yet handle well, like knowing about a details include. Change-Id: Ie3744f2c8cba68a8700b406449d6c2018a736952 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-27Enable clang-tidy-18 misc checksEd Tanous1-1/+8
Enable the checks we pass already. This also removes the commented out misc-no-recursion, considering we don't pass it. Tested: Clang-tidy passes. Change-Id: Ibaed95677aed85188bff483d2cd53605faaf7cc6 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-27Add clang-tidy-18 bugprone checksEd Tanous1-0/+13
Another clang version, another set of checks we can enable. bmcweb passes all these checks today, so enable them to help folks write better code. Change-Id: Ied6a364ee92d8d634edea717cfa2fb5245d534f9 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-27Check optionals in tidyEd Tanous1-0/+1
clang-tidy-18 makes this feature stable enough for us to use in general. Enable the check, and fix the couple of regressions that have snuck in since we last ran the check. Tidy seems to not be able to understand that ASSERT will not continue, so if we ASSERT a std::optional, it's not a bug. Add explicit checks to keep tidy happy. Tested: clang-tidy passes. Change-Id: I0986453851da5471056a7b47b8ad57a9801df259 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-25Enable bugprone clang checkEd Tanous1-0/+1
bugprone-multi-level-implicit-pointer-conversion is something that we pass currently, with one exception in the deprecated rest API. Ignore the one exception, as it's not clear how to fix it, and enable the check. Tested: Clang tidy passes. Change-Id: Idc10e0bb7b876e1c70afa28f9c27cc7bef1db0d7 Signed-off-by: Ed Tanous <ed@tanous.net>
2023-07-20Replace logging with std::formatEd Tanous1-0/+1
std::format is a much more modern logging solution, and gives us a lot more flexibility, and better compile times when doing logging. Unfortunately, given its level of compile time checks, it needs to be a method, instead of the stream style logging we had before. This requires a pretty substantial change. Fortunately, this change can be largely automated, via the script included in this commit under scripts/replace_logs.py. This is to aid people in moving their patchsets over to the new form in the short period where old patches will be based on the old logging. The intention is that this script eventually goes away. The old style logging (stream based) looked like. BMCWEB_LOG_DEBUG << "Foo " << foo; The new equivalent of the above would be: BMCWEB_LOG_DEBUG("Foo {}", foo); In the course of doing this, this also cleans up several ignored linter errors, including macro usage, and array to pointer deconstruction. Note, This patchset does remove the timestamp from the log message. In practice, this was duplicated between journald and bmcweb, and there's no need for both to exist. One design decision of note is the addition of logPtr. Because the compiler can't disambiguate between const char* and const MyThing*, it's necessary to add an explicit cast to void*. This is identical to how fmt handled it. Tested: compiled with logging meson_option enabled, and launched bmcweb Saw the usual logging, similar to what was present before: ``` [Error include/webassets.hpp:60] Unable to find or open /usr/share/www/ static file hosting disabled [Debug include/persistent_data.hpp:133] Restored Session Timeout: 1800 [Debug redfish-core/include/event_service_manager.hpp:671] Old eventService config not exist [Info src/webserver_main.cpp:59] Starting webserver on port 18080 [Error redfish-core/include/event_service_manager.hpp:1301] inotify_add_watch failed for redfish log file. [Info src/webserver_main.cpp:137] Start Hostname Monitor Service... ``` Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: I86a46aa2454be7fe80df608cb7e5573ca4029ec8
2023-07-11Add clang-analyzer checks from clang-16Ed Tanous1-0/+5
We pass all of these checks just fine (probably because we compile with clang on a regular basis). Enable the new checks. Tested: Clang-tidy passes. Change-Id: I493143c8b4d3a348fba277ade3bb97f6cf9d270a Signed-off-by: Ed Tanous <edtanous@google.com>
2023-07-11Add contains type tidy checkEd Tanous1-0/+1
On general container usage, contains() is more descriptive than count() > 0. We have one violation of this, fix it and enable the check. Tested: Clang-tidy passes. Change-Id: Ib5702ef97c6da033b6587c9cfebbe30dfbfe80b4 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-07-11Implement data pointer clang-tidy checkEd Tanous1-0/+1
readability-container-data-pointer flags one error in our codebase, but can definitely find issues in patchsets. Fix the one error (that came from crow), and enable the check. Change-Id: I3045ec9a58d80300c90921dda1a2fe3859ffed7b Signed-off-by: Ed Tanous <edtanous@google.com>
2023-07-11Enable bugprone clang tidy checksEd Tanous1-0/+7
We pass basically all of these with flying colors now. Previously there were clang-tidy bugs around bugprone-exception-escape, but those seem to be resolved in clang-tidy-16, so we can turn that check back on. The only thing we had was an instance of bugprone-assignment-in-if-condition, that was trivially fixed. Tested: Clang-tidy passes. Change-Id: Ib2e63e951ee8e4e46f0d1b40bf41f8dca59c9a08 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-07-10Tidy enable modernize-redundant-void-argEd Tanous1-0/+1
We have places where we explicitly set something to the pattern of method(void) This is no longer necessary to declare, so fix the places where we do it to make the codebase consistent, and enable the check. Tested: Clang-tidy passes. Change-Id: I3ef03fc07d65b656fecbcfea638dd12ba95f22e0 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-03-15Take url views by valueEd Tanous1-0/+1
Any of our things taking URLs should be taking url_view by value, similar to how we take string_view. From the beast documentation: "...it acts like a string_view in terms of ownership." [1] Therefore, we should treat it like we treat string_view, and take by value, not reference. [1] https://www.boost.org/doc/libs/master/libs/url/doc/html/url/ref/boost__urls__url_view.html Tested: Stacked these patches. Redfish service validator passes. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I696b495f4aa04984225853f653cc175c0eaad79d
2023-01-17Add check for globalsEd Tanous1-0/+1
We don't follow this cpp core guidelines rule well. This is something that we should aspire to cleaning up in the future, but for the moment, lets turn the rule on in clang-tidy to stop the bleeding, add ignores for the things that we know need some better abstractions, and work on these over time. Most of this commit is just adding NOLINTNEXTLINE exceptions for all of our globals. There was one case in the sensor code where clang correctly noted that those globals weren't actually const, which got missed because of the use of auto. Tested: CI should be good enough for this. Passes clang-tidy. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ieda08fee69a3b209d4b3e9771809a6c41524f066
2022-12-15Prepare for boost::url upgradeEd Tanous1-1/+0
The new boost URL now interops properly with std::string_view, which is great, and cleans up a bunch of mediocre code to convert one to another. It has also been pulled into boost-proper, so we no longer need a boost-url dependency that's separate. Unfortunately, boost url makes these improvements by changing boost::string_view for boost::urls::const_string, which causes us to have some compile errors on the missing type. The bulk of these changes fall into a couple categories, and have to be executed in one commit. string() is replaced with buffer() on the url and url_view types boost::string_view is replaced by std::string_view for many times, in many cases removing a temporary that we had in the code previously. Tested: Code compiles with boost 1.81.0 beta. Redfish service validator passes. Pretty good unit test coverage for URL-specific use cases. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I8d3dc89b53d1cc390887fe53605d4867f75f76fd
2022-12-08Generate Redfish enums from schemasEd Tanous1-0/+1
OpenBMC tends to have a significant problem in doing the appropriate lookups from the schema files, and many bugs have been injected by users picking a bad enum, or mistyping the casing of an enum value. At the same time, nlohmann::json has recently added first class support for enums, https://json.nlohmann.me/features/enum_conversion/ This commit attempts to build a set of redfish includes file with all the available Redfish enums in an easy to use enum class. This makes it very clear which enums are supported by the schemas we produce, and adds very little to no extra boilerplate on the human-written code we produced previously. Note, in the generated enum class, because of our use of the clang-tidy check for macros, the clang-tidy check needs an exception for these macros that don't technically follow the coding standard. This seems like a reasonable compromise, and in this case, given that nlohmann doesn't support a non-macro version of this. One question that arises is what this does to the binary size.... Under the current compiler optimizations, and with the current best practices, it leads to an overall increase in binary size of ~1200 bytes for the enum machinery, then approximately 200 bytes for every call site we switch over. We should decide if this nominal increase is reasonable. Tested: Redfish protocol validator runs with same number of failures as previously. Redfish Service Validator passes (one unrelated qemu-specific exception) Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I7c7ee4db0823f7c57ecaa59620b280b53a46e2c1
2022-06-30Require explicit decorator on one arg constructorsEd Tanous1-0/+1
We essentially follow this rule already, not relying on implicit operators, although there are a number of cases where in theory we could've implicitly constructed an object. This commit enables the clang-tidy check. Tested: Code compiles, passes clang-tidy. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ia428463313b075c69614fdb326e8c5c094e7adde
2022-03-12Enable readability checksEd Tanous1-2/+14
clang-tidy readability checks are overall a good thing, and help us to write consistent and readable code, even if it doesn't change the result. All changes done by the robot. Tested: Code compiles, inspection only (changes made by robot) Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iee4a0c74a11eef9f158f0044eae675ebc518b549
2022-03-09Enable 3 member function checksEd Tanous1-0/+3
The only changes were to make some functions static, which is essentially no-op. Changes were done by the robot. Tested: Unit tests pass, changes no-op Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id84ca2bee6f237877ba2900b2cbe4679b38a91dc
2022-02-15Enable readability-uppercase-literal-suffixEd Tanous1-1/+2
We only had a few violations of this; Fix them and enable the check. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I159e774fd0169a91a092218ec8dc896ba9edebf4
2022-02-15readability-static-accessed-through-instanceEd Tanous1-0/+1
We access std::string::npos through member variables in a couple places. Fix it. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I587f89e1580661aa311dfe4e06591ab38806e241
2022-02-15Enable readability-implicit-bool-conversion checksEd Tanous1-0/+1
These checks ensure that we're not implicitly converting ints or pointers into bools, which makes the code easier to read. Tested: Ran series through redfish service validator. No changes observed. UUID failing in Qemu both before and after. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I1ca0be980d136bd4e5474341f4fd62f2f6bbdbae
2022-02-11Enable performance-no-int-to-ptr checkEd Tanous1-0/+1
We have no violations of this. Just turn on the check. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie27f5b99fc96b4e61c063bf7979697f3d738744a
2022-02-11Add readability-redundant-* checksEd Tanous1-0/+7
There's a number of redundancies in our code that clang can sanitize out. Fix the existing problems, and enable the checks. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie63d7b7f0777b702fbf1b23a24e1bed7b4f5183b
2022-02-09Enable readability-avoid-const-params-in-declsEd Tanous1-0/+1
This check involves explicitly declaring variables const when they're declared auto, which helps in readability, and makes it more clear that the variables are const. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I71198ea03850384a389a56ad26f2c4a48c75b148
2022-02-07Enable readability-redundant-control-flow checksEd Tanous1-0/+1
These checks are a nice addition to our static analysis, as they simplify code quite a bit, as can be seen by this diff being negative lines. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I60ede4ad23d7e5337e811d70ddcab24bf8986891
2022-02-07Enable readability-named-parameter checksEd Tanous1-0/+1
We don't have too many violations here, probably because we don't have many optional parameters. Fix the existing instances, and enable the check. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I4d512f0ec90b060fb60a42fe3cd6ba72fb6c6bcb
2022-01-28Enable readability-container-size-empty testsEd Tanous1-0/+1
This one is a little trivial, but it does help in readability. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I5366d4eec8af2f781b3bad804131ae2eb806e3aa
2022-01-12Enable and enforce cpp core guidelinesEd Tanous1-0/+8
While in theory we enforce cpp core guidelines in code review, clearly, we're missing some things. This commit enables all the automated checks that we can pass reasonably at the moment. Tested: Code compiles Tested: Ran redfish service validator on the result of the series. No new failures. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I6dca1c822de38931eb9fa252db99fb57401b5a46
2022-01-12Enable pro-type-static-cast-downcast checksEd Tanous1-0/+1
We only had one usage, and it was really bad and breaking const correctness, so fix it to read the crashdump in inline. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I234946fe09d73a9fa0191a15a89d0b2c26f32337
2022-01-12Enable malloc checksEd Tanous1-0/+1
We only use malloc in one place, when we hand a pointer off to PAM. Ignore that one issue, and enable the check. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I37c41c193bae1bab370b03944617c642df0179fc
2022-01-12enable cppcoreguidelines-pro-type-vararg checksEd Tanous1-0/+1
We only use varargs in some code we borrowed from nlohmann, so ignore that, and enable the check. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iab1305784e7532e2ee10c617fb59b75aba142ce6
2022-01-12Enable pro-type-cstyle-cast checksEd Tanous1-0/+1
We actually do a pretty good job of this, and only have one C style cast, that's part of an openssl macro, so ignore the one, and enable the checks. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie0462ee947c8310457365ba2aeea78caedb93da1
2022-01-12Enable const_cast checksEd Tanous1-0/+1
const_cast is an anti pattern. There are a few places we need to do it for interacting with C APIs, so enable the checks, and ignore the existing uses. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: If1748213992b97f5e3e04cf9b86a6fcafbb7cf06
2022-01-12Enable pointer devolution checksEd Tanous1-0/+1
Enable cpp core guidelines checks for pointer deevolution. For the moment, simply ignore the uses, although ideally these should be cleaned up at some point. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I9a8aae94cc7a59529eab89225a37e89628c17597
2022-01-12Enable cpp core guidelines macro checksEd Tanous1-0/+1
We only use a couple macros. Ignore them in the checks. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I38feb10f76f6aaea8899617f081c9be68c88b3eb
2022-01-12Enforce variable initEd Tanous1-0/+2
There were a few places we weren't initting our variables per cpp core guidelines. Fix all of them, and enable checks for this. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iba09924beb9fb26f597ff94d1cecbd6d6b1af912
2022-01-12Enable checks for pointer arithmeticEd Tanous1-0/+1
Quite a few places we've disobeyed this rule, so simply ignore them for now to avoid new issues popping up. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I3e518a8e8742279afb3ad1a9dad54006ed109fb1
2022-01-12Enable reinterpre_cast checksEd Tanous1-0/+1
We seem to use reinterpret cast in a few cases unfortunately. For the moment, simply ignore most of them, and make it so we don't get more. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ic860cf922576b18cdc8d51d6132f5a9cbcc1d9dc
2022-01-12Enable cppcoreguidelines-special-member-functions checksEd Tanous1-0/+1
Part of enforcing cpp core guidelines involves explicitly including all constructors required on a non-trivial class. We were missing quite a few. In all cases, the copy/move/and operator= methods are simply deleted. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie8d6e8bf2bc311fa21a9ae48b0d61ee5c1940999
2022-01-12Enable init checkerEd Tanous1-0/+1
clang-tidy added cppcoreguidelines-init-variables as a check, which is something we already enforce to some extent, but getting CI to enforce it will help reviews move faster. Tested: Code compiles. Noop changes. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I7e10950de617b1d3262265572b1703f2e60b69d0
2022-01-12Enable clang-tidy checks we already passEd Tanous1-1/+10
clang-13 brought some additional checks we can turn on that we already pass, so enable them. List of checks can be found in the diff, and includes the suspicious includes check, which we previously had to disable due to a clang bug. Tested: Code compiles, clang-tidy passes Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I1a4d1da0a8e775cdeb6b898bc1cdb0f3f7b6f06a
2022-01-12Enable bugprone widening checks in clangEd Tanous1-0/+1
Most of the errors we hit are simply places we need to explicitly increase the width of the integer. Luckily, these are few and far between. Tested: Code compiles, unit tests pass. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I617d87f3970ae773e0767bb2f20118fca2e71daa
2022-01-12enable bugprone exception escape checkEd Tanous1-0/+1
clang-13 includes new checks, and finds some issues. The first is that the boost::vector constructor can possibly throw, so replace the underlying flat_map container with std::vector instead. The others are places where we could possibly throw in destructors, which would be bad. Ideally we wouldn't use the destructor pattern, but that would be non-trivial to clean up at this point, so just catch the exception, and log it. At the same time, catch exceptions thrown to main and log them. Tested: Code compiles Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I77b86eaa2fc79e43d1ca044c78ca3b0ce0a7c38c
generated by cgit v1.2.3 (git 2.39.0) at 2024-05-17 14:40:54 +0300