Enable clang-include-cleaner

clang-include-cleaner was added in clang-18. Enable it so we can
automatically find missing or overly broad includes.

Tested: clang-tidy passes.

Change-Id: I921737d18

Enable clang-include-cleaner

clang-include-cleaner was added in clang-18. Enable it so we can
automatically find missing or overly broad includes.

Tested: clang-tidy passes.

Change-Id: I921737d18b80920f91e36a8bb52f94497032847d
Signed-off-by: Ed Tanous <etanous@nvidia.com>

show more ...

Add new tidy checks

Another clang, another list of checks we can enable.

Tested: Clang-tidy passes.

Change-Id: Ib3fcd8046ce9f2caf9f9d95571ffa3cc2f56c596
Signed-off-by: Ed Tanous <ed@tanous.net>

Add missing headers

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 the

Add missing headers

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>

show more ...

Fix large copies with url_view and segments_view

Despite these objects being called "view" they are still relatively
large, as clang-tidy correctly flags, and we ignore.

Change all function uses to

Fix large copies with url_view and segments_view

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>

show more ...

Rework logger to create compile time errors

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 l

Rework logger to create compile time errors

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>

show more ...

Move where appropriate

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: I0cf58204

Move where appropriate

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>

show more ...

Enable readability check

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 genera

Enable readability check

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>

show more ...

Fix redundant inline operators

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: I3115b7c0

Fix redundant inline operators

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>

show more ...

Add misc-include-cleaner

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
w

Add misc-include-cleaner

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>

show more ...

Enable clang-tidy-18 misc checks

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: Ibae

Enable clang-tidy-18 misc checks

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>

show more ...

Add clang-tidy-18 bugprone checks

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: Ied6a

Add clang-tidy-18 bugprone checks

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>

show more ...

Check optionals in tidy

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.

Check optionals in tidy

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>

show more ...

Enable bugprone clang check

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 no

Enable bugprone clang check

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>

show more ...

Replace logging with std::format

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

Replace logging with std::format

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

show more ...

Add clang-analyzer checks from clang-16

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

Add clang-analyzer checks from clang-16

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>

show more ...

Add contains type tidy check

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.

Ch

Add contains type tidy check

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>

show more ...

Implement data pointer clang-tidy check

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

Implement data pointer clang-tidy check

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>

show more ...

Enable bugprone clang tidy checks

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 cla

Enable bugprone clang tidy checks

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>

show more ...

Tidy enable modernize-redundant-void-arg

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

Tidy enable modernize-redundant-void-arg

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>

show more ...

Take url views by value

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 o

Take url views by value

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

show more ...

Add check for globals

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-ti

Add check for globals

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

show more ...

Prepare for boost::url upgrade

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 pulle

Prepare for boost::url upgrade

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

show more ...

Generate Redfish enums from schemas

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,

Generate Redfish enums from schemas

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

show more ...

Require explicit decorator on one arg constructors

We essentially follow this rule already, not relying on implicit
operators, although there are a number of cases where in theory we
could've implic

Require explicit decorator on one arg constructors

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

show more ...

Enable readability checks

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 ro

Enable readability checks

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

show more ...