clang-format: re-format for clang-18

clang-format-18 isn't compatible with the clang-format-17 output, so we
need to reformat the code with the latest version. The way clang-18
handles lambda forma

clang-format: re-format for clang-18

clang-format-18 isn't compatible with the clang-format-17 output, so we
need to reformat the code with the latest version. The way clang-18
handles lambda formatting also changed, so we have made changes to the
organization default style format to better handle lambda formatting.

See I5e08687e696dd240402a2780158664b7113def0e for updated style.
See Iea0776aaa7edd483fa395e23de25ebf5a6288f71 for clang-18 enablement.

Change-Id: Iceec1dc95b6c908ec6c21fb40093de9dd18bf11a
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>

show more ...

Move logging args

Args captured by logging functions should be captured by rvalue, and use
std::forward to get perfect forwarding. In addition, separate out the
various std::out lines.

While we're

Move logging args

Args captured by logging functions should be captured by rvalue, and use
std::forward to get perfect forwarding. In addition, separate out the
various std::out lines.

While we're here, also try to optimize a little. We should ideally be
writing each log line to the output once, and ideally not use iostreams,
which induce a lot of overhead.

Similar to spdlog[1] (which at one point this codebase used), construct
the string, then call fwrite and fflush once, rather than calling
std::cout repeatedly.

Now that we don't have a dependency on iostreams anymore, we can remove
it from the places where it has snuck in.

Tested:
Logging still functions as before. Logs present.

[1] https://github.com/gabime/spdlog/blob/27cb4c76708608465c413f6d0e6b8d99a4d84302/include/spdlog/sinks/stdout_sinks-inl.h#L70C7-L70C13

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

show more ...

Bring consistency to config options

The configuration options that exist in bmcweb are an amalgimation of
CROW options, CMAKE options using #define, pre-bmcweb ifdef mechanisms
and meson options usi

Bring consistency to config options

The configuration options that exist in bmcweb are an amalgimation of
CROW options, CMAKE options using #define, pre-bmcweb ifdef mechanisms
and meson options using a config file. This history has led to a lot of
different ways to configure code in the codebase itself, which has led
to problems, and issues in consistency.

ifdef options do no compile time checking of code not within the branch.
This is good when you have optional dependencies, but not great when
you're trying to ensure both options compile.

This commit moves all internal configuration options to:
1. A namespace called bmcweb
2. A naming scheme matching the meson option. hyphens are replaced with
underscores, and the option is uppercased. This consistent transform
allows matching up option keys with their code counterparts, without
naming changes.
3. All options are bool true = enabled, and any options with _ENABLED or
_DISABLED postfixes have those postfixes removed. (note, there are
still some options with disable in the name, those are left as-is)
4. All options are now constexpr booleans, without an explicit compare.

To accomplish this, unfortunately an option list in config/meson.build
is required, given that meson doesn't provide a way to dump all options,
as is a manual entry in bmcweb_config.h.in, in addition to the
meson_options. This obsoletes the map in the main meson.build, which
helps some of the complexity.

Now that we've done this, we have some rules that will be documented.
1. Runtime behavior changes should be added as a constexpr bool to
bmcweb_config.h
2. Options that require optionally pulling in a dependency shall use an
ifdef, defined in the primary meson.build. (note, there are no
options that currently meet this class, but it's included for
completeness.)

Note, that this consolidation means that at configure time, all options
are printed. This is a good thing and allows direct comparison of
configs in log files.

Tested: Code compiles
Server boots, and shows options configured in the default build. (HTTPS,
log level, etc)

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

show more ...

Fix moves/forward

Clang has new checks for std::move/std::forward correctness, which
catches quite a few "wrong" things where we were making copies of
callback handlers.

Unfortunately, the lambda s

Fix moves/forward

Clang has new checks for std::move/std::forward correctness, which
catches quite a few "wrong" things where we were making copies of
callback handlers.

Unfortunately, the lambda syntax of

callback{std::forward<Callback>(callback)}

in a capture confuses it, so change usages to
callback = std::forward<Callback>(callback)

to be consistent.

Tested: Redfish service validator passes.

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

show more ...

clang-format: copy latest and re-format

clang-format-17 has some backwards incompatible changes that require
additional settings for best compatibility and re-running the formatter.
Copy the latest

clang-format: copy latest and re-format

clang-format-17 has some backwards incompatible changes that require
additional settings for best compatibility and re-running the formatter.
Copy the latest .clang-format from the docs repository and reformat the
repository.

Change-Id: I2f9540cf0d545a2da4d6289fc87b754f684bc9a7
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>

show more ...

Move http client to URL

Type safety is a good thing. In:
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/65606

It was found that splitting out the URI into encoded pieces in the early
phase removed

Move http client to URL

Type safety is a good thing. In:
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/65606

It was found that splitting out the URI into encoded pieces in the early
phase removed some information we needed, namely whether or not a URI
was ipv6. This commit changes http client such that it passes all the
information through, with the correct type, rather than passing in
hostname, port, path, and ssl separately.

Opportunistically, because a number of log lines are changing, this uses
the opportunity to remove a number of calls to std::to_string, and rely
on std::format instead.

Now that we no longer use custom URI splitting code, the
ValidateAndSplitUrl() method can be removed, given that our validation
now happens in the URI class.

Tested: Aggregation works properly when satellite URIs are queried.

Change-Id: I9f605863179af54c5af2719bc5ce9d29cbfffab7
Signed-off-by: Ed Tanous <edtanous@google.com>

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

AsyncResolve cleanups and error handling

The Async DBus resolver really has nothing to do with crow, which is our
core http library namespace and has some opportunistic cleanups that can
be done.

T

AsyncResolve cleanups and error handling

The Async DBus resolver really has nothing to do with crow, which is our
core http library namespace and has some opportunistic cleanups that can
be done.

This commit moves it into the bmcweb namespace (unimportantly) and
breaks out one of the larger functions such that it can be unit tested,
and unit tests it.

Tested: Unit tests pass.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie3cfbb0ef81a027a1ad42358c04967a517471117

show more ...

Allow async resolver to be optional

This commit adds a meson option to allow selecting which dns resolver
bmcweb uses. There are use cases, like Open Compute Project Inband
Management Agent, that w

Allow async resolver to be optional

This commit adds a meson option to allow selecting which dns resolver
bmcweb uses. There are use cases, like Open Compute Project Inband
Management Agent, that would require not using dbus, which would require
us to fall back to the asio resolver. This commit makes the existing
asio resolver constructor, and async_resolve methods match the
equivalents in asio (which we intended to do anyway), then adds a macro
and configure option for being able to select which resolver backend to
rely on.

Tested: Code can now compile without sdbusplus.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I3220214367179f131a60082bdfaf7e725d35c125

show more ...

Take boost error_code by reference

By convention, we should be following boost here, and passing error_code
by reference, not by value. This makes our code consistent, and removes
the need for a co

Take boost error_code by reference

By convention, we should be following boost here, and passing error_code
by reference, not by value. This makes our code consistent, and removes
the need for a copy in some cases.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Id42ea4a90b6685a84818b87d1506c11256b3b9ae

show more ...

Fix a boatload of #includes

Most of these missing includes were found by running clang-tidy on all
files, including headers. The existing scripts just run clang-tidy on
source files, which doesn't

Fix a boatload of #includes

Most of these missing includes were found by running clang-tidy on all
files, including headers. The existing scripts just run clang-tidy on
source files, which doesn't catch most of these.

Tested: Code compiles

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ic741fbb2cc9e5e92955fd5a1b778a482830e80e8

show more ...

Aggregation: Detect and fix all URI properties

There are a number of properties of Type "string (uri)" for which we
do not currently support adding prefixes. This patch adds support
for all existin

Aggregation: Detect and fix all URI properties

There are a number of properties of Type "string (uri)" for which we
do not currently support adding prefixes. This patch adds support
for all existing URI properties which are missed by the existing
implementation.

This change will be needed by future patches which will expand
aggregation support to all top level collections defined by the
schema. Those collections that are not currently supported include
properties whose URIs should be fixed, but would be missed by the
existing implementation.

Tested:
New unit test passes.
URI properties are still handled correctly.

```shell
curl localhost/redfish/v1/Chassis/5B247A_<chassisID>
{
"@odata.id": "/redfish/v1/Chassis/5B247A_<chassisID>",
"@odata.type": "#Chassis.v1_16_0.Chassis",
"Actions": {
"#Chassis.Reset": {
"@Redfish.ActionInfo": "/redfish/v1/Chassis/5B247A_<chassisID>/ResetActionInfo",
"target": "/redfish/v1/Chassis/5B247A_<chassisID>/Actions/Chassis.Reset"
}
},
...
}
```

Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: I3b3e06ee3191564d266598f7bc9f1641e6fcb333

show more ...

Try to fix the lambda formatting issue

clang-tidy has a setting, LambdaBodyIndentation, which it says:
"For callback-heavy code, it may improve readability to have the
signature indented two levels

Try to fix the lambda formatting issue

clang-tidy has a setting, LambdaBodyIndentation, which it says:
"For callback-heavy code, it may improve readability to have the
signature indented two levels and to use OuterScope."

bmcweb is very callback heavy code. Try to enable it and see if that
improves things. There are many cases where the length of a lambda call
will change, and reindent the entire lambda function. This is really
bad for code reviews, as it's difficult to see the lines changed. This
commit should resolve it. This does have the downside of reindenting a
lot of functions, which is unfortunate, but probably worth it in the
long run.

All changes except for the .clang-format file were made by the robot.

Tested: Code compiles, whitespace changes only.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib4aa2f1391fada981febd25b67dcdb9143827f43

show more ...

Remove regex uses in event service and consolidate

As the patch at
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/50994 can attest,
parsing urls with a regex is error prone. We should avoid

Remove regex uses in event service and consolidate

As the patch at
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/50994 can attest,
parsing urls with a regex is error prone. We should avoid it where
possible, and we have boost::urls that implements a full, correct, and
unit tested parser.

Ideally, eventually this helper function would devolve into just the
parse_uri, and setting defaults portion, and we could rely on the
boost::urls::url class to pass into things like http_client.

As a side note, because boost url implements port as a proper type-safe
uint16, some interfaces that previously accepted port by std::string&
needed to be modified, and is included in this patch.

Also, once moved, the branch on the ifdef for HTTP push support was
failing a clang-tidy validation. This is a known limitation of using
ifdefs for our code, and something we've solved with the header file, so
move the http push enabler to the header file.

Also note that given this reorganization, two EXPECT statements are
added to the unit tests for user input behaviors that the old code
previously did not handle properly.

Tested: Unit tests passing

Ran Redfish-Event-Listener, saw subscription create properly:
Subcription is successful for https://192.168.7.2, /redfish/v1/EventService/Subscriptions/2197426973

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ia4127c6cbcde6002fe8a50348792024d1d615e8f

show more ...

Don't rely on operator << for object logging

In the upcoming fmt patch, we remove the use of streams, and a number of
our logging statements are relying on them. This commit changes them to
no long

Don't rely on operator << for object logging

In the upcoming fmt patch, we remove the use of streams, and a number of
our logging statements are relying on them. This commit changes them to
no longer rely on operator>> or operator+ to build their strings. This
alone isn't very useful, but in the context of the next patch makes the
automation able to do a complete conversion of all log statements
automatically.

Tested: enabled logging on local and saw log statements print to console

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I0e5dc2cf015c6924037e38d547535eda8175a6a1

show more ...

Enable checks for pointer arithmetic

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

Enable checks for pointer arithmetic

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

show more ...

Enable cppcoreguidelines-special-member-functions checks

Part of enforcing cpp core guidelines involves explicitly including all
constructors required on a non-trivial class. We were missing quite

Enable cppcoreguidelines-special-member-functions checks

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

show more ...

Enable init checker

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.

T

Enable init checker

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

show more ...

Enable clang-tidy forward reference checks

Clang-13 adds new checks we can turn on, which find quite a few errors.

Tested: Code compiles

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I

Enable clang-tidy forward reference checks

Clang-13 adds new checks we can turn on, which find quite a few errors.

Tested: Code compiles

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I74b780760014c898cc440b37aea640b33e91c439

show more ...

EventService : Support async_resolve for subscribers

The http client at bmcweb does not resolve the client's
hostname asynchronously

This commit implements the async_resolve by using systemd resolv

EventService : Support async_resolve for subscribers

The http client at bmcweb does not resolve the client's
hostname asynchronously

This commit implements the async_resolve by using systemd resolved.
The async dbus message to resolvd.service is sent when a subscriber
successfully subscribes for events. The method ResolveHostname is
used to resolve the subscriber's hostname

Tested by:
Subscribe for the events at BMC using DMTF event listener
Generate an event and see the same is received at the listener's console

Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
Change-Id: I3ab8206ac4764cfa025e94c06407524d6ba220e0

show more ...