Remove logically dead code

This code path is subtle, but given that slotPresent is only set to true
if totalCores is incremented, there's no way to actually hit this
section of code.

Looking for in

Remove logically dead code

This code path is subtle, but given that slotPresent is only set to true
if totalCores is incremented, there's no way to actually hit this
section of code.

Looking for input on if this is the right behavior.

Change-Id: Ie6dadd2c7a0ca6b8402148ddd9b8a369a4a38b2e
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 ...

Fix account service

Session might not be initialized, and might be nullptr.

This line was accessing the session BEFORE the nullptr check. Move it
to after. Found using static analysis.

Change-Id

Fix account service

Session might not be initialized, and might be nullptr.

This line was accessing the session BEFORE the nullptr check. Move it
to after. Found using static analysis.

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

show more ...

Move to Redfish Action specific setProperty call

This commit will migrate all the setProperty calls initiated by a
redfish"Action" to "setDbusProperty" method in Redfish namespace that
handles all D

Move to Redfish Action specific setProperty call

This commit will migrate all the setProperty calls initiated by a
redfish"Action" to "setDbusProperty" method in Redfish namespace that
handles all DBuserrors in a consistent manner.

This method will determine if a setProperty is called during redfish
"Action" or just setting of a dbus property and internally call
appropriate methods that handles different set of errors.

All the Redfish action specific errors are defined in error_messages.hpp
file.
This specific change moves setProperty call in hypervisor_system.hpp and
covers errors in the mentioned file only.

Tested-By:
<Yet to test this usecase>

Change-Id: I3da48fbeabcdcf088c4481021232f08a44797c86
Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>
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 ...

Remove redundant static modifier

"inline static void func()"

Doesn't make sense when put in a header file. Find all instances, and
make them inline like we do everywhere else.

Tested: Code compil

Remove redundant static modifier

"inline static void func()"

Doesn't make sense when put in a header file. Find all instances, and
make them inline like we do everywhere else.

Tested: Code compiles.

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

show more ...

Fix undefined behavior in getUniqueEntryID

Changing
static bool getUniqueEntryID
to
inline bool getUniqueEntryID

Exposes the fact that there's some undefined behavior here, and unit
tests start fa

Fix undefined behavior in getUniqueEntryID

Changing
static bool getUniqueEntryID
to
inline bool getUniqueEntryID

Exposes the fact that there's some undefined behavior here, and unit
tests start failing, likely due to stack being reused where it
previously wasn't.

This commit cleans up the code to simplify it, and remove the problem.

Tested: Unit tests pass. Good coverage.

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

show more ...

Remove redfish-health-populate

The redfish-health-populate option was scheduled to be removed in 1Q
2024. It is now 2Q, so remove the option. No upstream layers enabled it
and did not find a downstr

Remove redfish-health-populate

The redfish-health-populate option was scheduled to be removed in 1Q
2024. It is now 2Q, so remove the option. No upstream layers enabled it
and did not find a downstream layer that did either.

This was always limited to a few resources. Overall this design was only
half done. A future "HealthRollup" can be proposed.

Some discord discussion:
[1]: https://discord.com/channels/775381525260664832/855566794994221117/1110728560819327069

Commit disabling this (merged 10 months ago):
[2]: https://github.com/openbmc/bmcweb/commit/6f8273e49cffdd347c223b9538558edfb05e818a

Tested: Code compiles

Change-Id: I4d33c1e674ecdb0fd256df62f3795073454ae7a1
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

show more ...

Clean up Ethernet to use readJson

Today, patching ethernet ip address arrays can use several
styles.

IpAddresses: [{}, {value: value}, null]

All 3 of those elements are legal. Today, we unpack va

Clean up Ethernet to use readJson

Today, patching ethernet ip address arrays can use several
styles.

IpAddresses: [{}, {value: value}, null]

All 3 of those elements are legal. Today, we unpack values like that
with nlohmann::json, then iterate and unpack further. This leads to
problems where:

IpAddresses: [{}, {value: value}, 1.0]

would have the same behavior as the prior, given that we check for
"is_object()" to determine the null state. This is messy at best, and
not typesafe at worst.

Changing this code to use the new class NullOr<> allows the readJson
parser to fail the second example.

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

show more ...

Clean up Account Service to use readJson

Use multiple level direct read.

Tested: Visual only. Need help if anyone wants to test.

Change-Id: I67c77bdd42a05a42f9cd1b40dc74517dceebdaad
Signed-off-by:

Clean up Account Service to use readJson

Use multiple level direct read.

Tested: Visual only. Need help if anyone wants to test.

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

show more ...

Clean up hypervisor to use readJson

Use multiple level direct read.

Tested: Visual only. Need help if anyone wants to test.

Change-Id: Ifcf716a2ba93fd565bbf134d4132532e60e3b4f0
Signed-off-by: Ed T

Clean up hypervisor to use readJson

Use multiple level direct read.

Tested: Visual only. Need help if anyone wants to test.

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

show more ...

Call dump() less

nlohmann::json::dump() is not an easy function to get the call
parameters correct on. We should limit the places we use it.

Luckily, both logging and redfish::messages support pri

Call dump() less

nlohmann::json::dump() is not an easy function to get the call
parameters correct on. We should limit the places we use it.

Luckily, both logging and redfish::messages support printing
json values directly. Use them where appropriate.

Tested: Error logging and out of range calls only of heavily used
messages and logging calls. Inspection only.

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

show more ...

log_services: Move to setProperty dbus util method

This commit changes sdbusplus setProperty calls in
log_services.hpp file to "setDbusProperty" method in Redfish
namespace that handles all DBus err

log_services: Move to setProperty dbus util method

This commit changes sdbusplus setProperty calls in
log_services.hpp file to "setDbusProperty" method in Redfish
namespace that handles all DBus errors in a consistent manner.

Change-Id: Icd9b0f0326c75a1421756d515408b303bdd738e3
Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>

show more ...

Remove redfish-enable-proccessor-memory-status

The redfish-enable-proccessor-memory-status option was scheduled to be
removed in 1Q 2024. It is now 2Q, so remove the option. No upstream
layers enabl

Remove redfish-enable-proccessor-memory-status

The redfish-enable-proccessor-memory-status option was scheduled to be
removed in 1Q 2024. It is now 2Q, so remove the option. No upstream
layers enabled it and I could not find a downstream layer that did
either.

Redfish deprecated the Processor/Memory Summary Status (state, health,
healthrollup) attributes.

Discussion on discord, when disabling:
[1]: https://discord.com/channels/775381525260664832/855566794994221117/1093939076710793296

Commit disabling this (merged 10 months ago):
[2]: https://github.com/openbmc/bmcweb/commit/5fd0aafb0f14fb3011970e8575647bb608688c7c

Tested: Code builds.

Change-Id: I539cd5f384633afa7badf1cecfc6c7a87062f672
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

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

hypervisor: Move to setProperty dbus util method

This commit changes sdbusplus setProperty calls in
hypervisor_system.hpp file to "setDbusProperty" method in Redfish
namespace that handles all DBus

hypervisor: Move to setProperty dbus util method

This commit changes sdbusplus setProperty calls in
hypervisor_system.hpp file to "setDbusProperty" method in Redfish
namespace that handles all DBus errors in a consistent manner.

Change-Id: Iebca5eb4e28159d61cd4b13c0343b78efd0f1f39
Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>

show more ...

Remove duplicated include

This include exists above.

Tested: code compiles.

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

Use no-switch-default on clang

clang-18 improves this check so that we can actually use it. Enable it
and fix all violations.

Change-Id: Ibe4ce19c423d447a4cbe593d1abba948362426af
Signed-off-by: Ed

Use no-switch-default on clang

clang-18 improves this check so that we can actually use it. Enable it
and fix all violations.

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

show more ...

Cable: Fix Logging Level Error

When we have an internal error, having the D-Bus response is really
helpful. Follow our guide and have these be a Logging Level Error.

Tested: None. Inspection only.

Cable: Fix Logging Level Error

When we have an internal error, having the D-Bus response is really
helpful. Follow our guide and have these be a Logging Level Error.

Tested: None. Inspection only.

Change-Id: Ie1d9f364c3af7f2a8839d878d68c82c10ddc0429
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

show more ...

ethernet: Move to setProperty dbus utility method

This commit changes sdbusplus setProperty calls in ethernet.hpp file
to "setDbusProperty" method in Redfish namespace that handles all DBus
errors i

ethernet: Move to setProperty dbus utility method

This commit changes sdbusplus setProperty calls in ethernet.hpp file
to "setDbusProperty" method in Redfish namespace that handles all DBus
errors in a consistent manner.

Tested By:
Tested a few PATCH operations on the redfish endpoints defined in
this file and verified that bmcweb returns appropriate Redfish
errors.

Change-Id: Ie456db75d59dc247cdce5dd5cc0b2f6894f5265f
Signed-off-by: Asmitha Karunanithi <asmitk01@in.ibm.com>

show more ...

Create Redfish specific setProperty call

There are currently 78 sdbusplus::asio::setProperty calls in
redfish-core. The error handler for nearly all of them looks something
like:

```
if (ec)
{

Create Redfish specific setProperty call

There are currently 78 sdbusplus::asio::setProperty calls in
redfish-core. The error handler for nearly all of them looks something
like:

```
if (ec)
{
const sd_bus_error* dbusError = msg.get_error();
if ((dbusError != nullptr) &&
(dbusError->name ==
std::string_view(
"xyz.openbmc_project.Common.Error.InvalidArgument")))
{
BMCWEB_LOG_WARNING("DBUS response error: {}", ec);
messages::propertyValueIncorrect(asyncResp->res, "<PropertyName>", <PropertyValue>);
return;
}
messages::internalError(asyncResp->res);
return;
}
messages::success(asyncResp->res);

```

In some cases there are more errors handled that translate to more error
messages, but the vast majority only handle InvalidArgument. Many of
these, like the ones in account_service.hpp, do the error handling in a
lambda, which causes readability problems. This commit starts to make
things more consistent, and easier for trivial property sets.

This commit invents a setDbusProperty method in the redfish namespace
that tries to handle all DBus errors in a consistent manner. Looking
for input on whether this will work before changing over the other 73
calls. Overall this is less code, fewer inline lambdas, and defaults
that should work for MOST use cases of calling an OpenBMC daemon, and
fall back to more generic errors when calling a "normal" dbus daemon.

As part of this, I've ported over several examples. Some things that
might be up in the air:
1. Do we always return 204 no_content on property sets? Today there's a
mix of 200, with a Base::Success message, and 204, with an empty body.
2. Do all DBus response codes map to the same error? A majority are
covered by xyz.openbmc_project.Common.Error.InvalidArgument, but there
are likely differences. If we allow any daemon to return any return
code, does that cause compatibility problems later?

Tested:
```
curl -k --user "root:0penBmc" -H "Content-Type: application/json" -X PATCH -d '{"HostName":"openbmc@#"}' https://192.168.7.2/redfish/v1/Managers/bmc/EthernetInterfaces/eth0
```

Returns the appropriate error in the response
Base.1.16.0.PropertyValueIncorrect

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

show more ...

Fix clang-formatting in ethernet

Current code has

//clang-format on

When it should have

// clang-format on

The difference is subtle, but disables formatting for this whole file.
Re-enable and fi

Fix clang-formatting in ethernet

Current code has

//clang-format on

When it should have

// clang-format on

The difference is subtle, but disables formatting for this whole file.
Re-enable and fix the couple of problems.

Tested: Whitespace only.

Change-Id: Ia155226327d4d611eb2c0f5232274459866e81cc
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 ...

Fix unused variable warning in LDAP

It's not clear how this came to be the way it is, but tidy now warns
that this variable is unused (which it is).

Refactor the LDAP code to not use the variable,

Fix unused variable warning in LDAP

It's not clear how this came to be the way it is, but tidy now warns
that this variable is unused (which it is).

Refactor the LDAP code to not use the variable, and to use concrete
object_t and array_t

Tested: Redfish service validator passes.

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

show more ...

Update indicator LED verification

Extend the hasIndicatorLed array and add
xyz.openbmc_project.Inventory.Item.Chassis interface.

Tested:
```
curl -k https://$bmc/redfish/v1/Chassis/chassis
{
"@od

Update indicator LED verification

Extend the hasIndicatorLed array and add
xyz.openbmc_project.Inventory.Item.Chassis interface.

Tested:
```
curl -k https://$bmc/redfish/v1/Chassis/chassis
{
"@odata.id": "/redfish/v1/Chassis/chassis",
"@odata.type": "#Chassis.v1_22_0.Chassis",
"Actions": {
"#Chassis.Reset": {
"@Redfish.ActionInfo": "/redfish/v1/Chassis/chassis/ResetActionInfo",
"target": "/redfish/v1/Chassis/chassis/Actions/Chassis.Reset"
}
},
"ChassisType": "RackMount",
"Id": "chassis",
"Links": {
"ComputerSystems": [
{
"@odata.id": "/redfish/v1/Systems/system"
}
],
"ManagedBy": [
{
"@odata.id": "/redfish/v1/Managers/bmc"
}
]
},
"Location": {
"PartLocation": {
"ServiceLabel": "U78DA.ND0.WZS004K"
}
},
"IndicatorLED": "Off",
"LocationIndicatorActive": false,
"Manufacturer": "",
"Model": "23",
"Name": "chassis",
"PCIeDevices": {
"@odata.id": "/redfish/v1/Systems/system/PCIeDevices"
},
"PartNumber": "",
"Power": {
"@odata.id": "/redfish/v1/Chassis/chassis/Power"
},
"PowerState": "Off",
"Sensors": {
"@odata.id": "/redfish/v1/Chassis/chassis/Sensors"
},
"SerialNumber": "",
"Status": {
"Health": "OK",
"HealthRollup": "OK",
"State": "StandbyOffline"
},
"Thermal": {
"@odata.id": "/redfish/v1/Chassis/chassis/Thermal"
}
}
```

Signed-off-by: George Liu <liuxiwei@ieisystem.com>
Change-Id: I02e77d56e555f9aee3f76015baeebbf1f4a292ab

show more ...