Fix some more includes

clang-tidy warns on these when run directly in a header file. Fix them.

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

Fix a couple #includes

In the continual quest to get tidy passing when run in isolation, fix
some more includes.

This includes removing a circular #include to app.hpp. We don't use
app.hpp in thes

Fix a couple #includes

In the continual quest to get tidy passing when run in isolation, fix
some more includes.

This includes removing a circular #include to app.hpp. We don't use
app.hpp in these files, which is why our code compiles but having this
include it here causes a few circular dependencies
app.hpp -> http_server.hpp -> persistent_data.hpp -> app.hpp.
app.hpp -> http_server.hpp -> authentication.hpp -> app.hpp.

This confuses clang when run on header files directly.

Fix a couple more includes at the same time.

Tested: Code compiles

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

show more ...

Refactor GetSubTree method

Since the GetSubTree method has been implemented in dbus_utility and
this commit is to integrate all the places where the GetSubTree
method is called, and use the method i

Refactor GetSubTree method

Since the GetSubTree method has been implemented in dbus_utility and
this commit is to integrate all the places where the GetSubTree
method is called, and use the method in dbus_utility uniformly.

Tested: Redfish Validator Passed

Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: If3852b487d74e7cd8f123e0efffbd4affe92743c

show more ...

Add the GetObject method to dbus_utility

There are currently many files that use the GetObject method.
Since they are a general method, they are defined in the
dbus_utility.hpp file and refactors th

Add the GetObject method to dbus_utility

There are currently many files that use the GetObject method.
Since they are a general method, they are defined in the
dbus_utility.hpp file and refactors them.

Tested: Built bmcweb successfully and Validator passes.

Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: If2af77294389b023b611987252ee6149906fcd25

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

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

Add missing const in update service

Per cpp core guidelines, this type should be const.

Tested: Code compiles. Trivial const change. cppcheck no longer warns
on this line.

Signed-off-by: Ed Tano

Add missing const in update service

Per cpp core guidelines, this type should be const.

Tested: Code compiles. Trivial const change. cppcheck no longer warns
on this line.

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

show more ...

Fix for incorrect Task Progress and State values

While performing the firmware updates, task will be created for tracking
the background update progress and state. There are error associated
with th

Fix for incorrect Task Progress and State values

While performing the firmware updates, task will be created for tracking
the background update progress and state. There are error associated
with that code currently.

Errors on TaskId's:
- Internal errors
"Messages": [
....
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The request failed due to an internal service error. The service is still operational.",
....
}
]

- Always task is in RUNNING state.
"PercentComplete": 0,
"TaskState": "Running",

Issue details:
- Percentage: The "Progress" property in D-Bus interface
"xyz.openbmc_project.Software.ActivationProgress" is of type
"uint8_t" but bmcweb is reading type as string and that is leading to
failure and invalid values.

- TaskState: TaskState is read from d-bus after recieving associated
signal but that is not updated properly and that caused the failure.
this commit corrects the same.

Tested:
After fix, task which is created for firmware update is showing correct
"PercentageComplete" and 'TaskState".

"PercentComplete": 100,
"TaskState": "Completed",

Signed-off-by: Gayathri Leburu <gayathri.leburu@intel.com>
Signed-off-by: moni kumar <monix.kumar@intel.com>
Signed-off-by: AppaRao Puli <apparao.puli@intel.com>
Change-Id: I0354cdecc9d9ddd47b017591402fd7b3149d12ca

show more ...

Bump timeout for code update file

This 10 second timeout is for the write, untar, inotify fire, and
creation of a software D-Bus object.
IBM has large images for its p10bmc, 125M and growing. We hav

Bump timeout for code update file

This 10 second timeout is for the write, untar, inotify fire, and
creation of a software D-Bus object.
IBM has large images for its p10bmc, 125M and growing. We have seen this
take longer than 10 seconds on a AST2600. Bump this timeout to 25
seconds.

Most of the 10 seconds seems to be the untar.

This default 10 seconds is only used for the http upload path. TFTP
already bumps this timeout to 600 seconds.

[1] https://github.com/openbmc/bmcweb/blob/master/redfish-core/lib/update_service.hpp#L488

Could have instead added the 25 seconds to the
monitorForSoftwareAvailable call but figured having the default be 25
is reasonable.

[1] https://github.com/openbmc/bmcweb/blob/master/redfish-core/lib/update_service.hpp#L530

On a fail:

```
Sep 07 12:15:01 p10bmc phosphor-version-software-manager[752]: Untaring /tmp/images/69fe6e22-2bd0-42c4-be69-7f388debc6d1 to /tmp/images/imageHaTO5K
Sep 07 12:15:08 p10bmc bmcweb[2299]: (2022-09-07 12:15:08) [ERROR "update_service.hpp":392] Timed out waiting for firmware object being created
Sep 07 12:15:08 p10bmc bmcweb[2299]: (2022-09-07 12:15:08) [ERROR "update_service.hpp":394] FW image may has already been uploaded to server
Sep 07 12:15:08 p10bmc bmcweb[2299]: (2022-09-07 12:15:08) [CRITICAL "error_messages.cpp":233] Internal Error ../git/redfish-core/include/../lib/update_service.hpp(403:49) `redfish::monitorForSoftwareAvailable(const std::shared_ptr<bmcweb::AsyncResp>&, const crow::Request&, const string&, int)::<lambda(const boost::system::error_code&)>`:
```

Tested: On a BMC hitting this, saw the Internal Error on upload without
this change. With this change, the update now works.

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

show more ...

clang-tidy: fix misc warnings

The following error reports have started to be reported by clang-tidy:

* readability-qualified-auto - add 'const' to `auto&` iterators
* bugprone-use-after-move -

clang-tidy: fix misc warnings

The following error reports have started to be reported by clang-tidy:

* readability-qualified-auto - add 'const' to `auto&` iterators
* bugprone-use-after-move - add break in loop after element is found

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

show more ...

used sdbusplus::unpackPropertiesNoThrow part 8

used sdbusplus::unpackPropertiesNoThrow in other places, also replaced
all usages of "GetAll" with sdbusplus::asio::getAllProperties

bmcweb size:

used sdbusplus::unpackPropertiesNoThrow part 8

used sdbusplus::unpackPropertiesNoThrow in other places, also replaced
all usages of "GetAll" with sdbusplus::asio::getAllProperties

bmcweb size: 2697640 -> 2685336 (-12304)
compressed size: 1129728 -> 1126078 (-3650)

Tested:
- Executed redfish service validator, no new errors detected

Change-Id: I916e462e004fcbde67c209daef295de8f5fb68eb
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>

show more ...

Update content of ResourceNotFound/ResourceAlreadyExists message

According to Redfish Base Message Registry definition [1], the first
argument of ResourceNotFound and ResourceAlreadyExists is the sc

Update content of ResourceNotFound/ResourceAlreadyExists message

According to Redfish Base Message Registry definition [1], the first
argument of ResourceNotFound and ResourceAlreadyExists is the schema
name of the resource. This patch changes the first argument to non-
versioned schema name treewide.

Tested:
Verified the error message matches the definition, and Redfish Service
Validator passed.

[1] https://redfish.dmtf.org/registries/Base.1.13.0.json

Change-Id: Ib5cd853578ef0bffda1184d10827241e94faaf68
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>

show more ...

sdbusplus: use shorter type aliases

The sdbusplus headers provide shortened aliases for many types.
Switch to using them to provide better code clarity and shorter
lines. Possible replacements are

sdbusplus: use shorter type aliases

The sdbusplus headers provide shortened aliases for many types.
Switch to using them to provide better code clarity and shorter
lines. Possible replacements are for:
* bus_t
* exception_t
* manager_t
* match_t
* message_t
* object_t
* slot_t

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

show more ...

Remove usages of boost::starts/ends_with

Per the coding standard, now that C++ supports std::string::starts_with
and std::string::ends_with, we should be using them over the boost
alternatives. Thi

Remove usages of boost::starts/ends_with

Per the coding standard, now that C++ supports std::string::starts_with
and std::string::ends_with, we should be using them over the boost
alternatives. This commit goes through and updates all usages.

Arguably some of these are incorrect, and instances of common error 13,
but because this is mostly a mechanical it intentionally doesn't try to
handle it.

Tested: Unit tests pass.

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

show more ...

Fix shadowed variable issues

This patchset is the conclusion of a multi-year effort to try to fix
shadowed variable names. Variables seem to be shadowed all over, and in
most places they exist, the

Fix shadowed variable issues

This patchset is the conclusion of a multi-year effort to try to fix
shadowed variable names. Variables seem to be shadowed all over, and in
most places they exist, there's a "code smell" of things that aren't
doing what the author intended.

This commit attempts to clean up these in several ways by:
1. Renaming variables where appropriate.
2. Preferring to refer to member variables directly when operating
within a class
3. Rearranging code so that pass through variables are handled in the
calling scope, rather than passing them through.

These patterns are applied throughout the codebase, to the point where
-Wshadow can be enabled in meson.build.

Tested: Code compiles, unit tests pass. Still need to run redfish
service validator.

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

show more ...

update_service: Refactor getSoftwareVersion into a seperate function

Moved the dbus call to SoftwareVersion into a function called
getSoftwareVersion to help organize the code for future supports/

update_service: Refactor getSoftwareVersion into a seperate function

Moved the dbus call to SoftwareVersion into a function called
getSoftwareVersion to help organize the code for future supports/

This help thin out the code a bit and will be a bit easier to navigate
the code.

Tested:
Passed Redfish Validation test for UpdateService.
No change to the redfish tree.

Change-Id: I3cbc8a9c79272fc86a453e528820b2b3b95476d3
Signed-off-by: Willy Tu <wltu@google.com>

show more ...

sw_util: Rename all Firmware to Software wherever possible

Rename to Software so we can reuse it for software inventory. This make
is more consistent to the SoftwareVersion dbus interface that is us

sw_util: Rename all Firmware to Software wherever possible

Rename to Software so we can reuse it for software inventory. This make
is more consistent to the SoftwareVersion dbus interface that is used.

Change-Id: I97fb10fccf744a6d6d4cba57f970759431bf4744
Signed-off-by: Willy Tu <wltu@google.com>

show more ...

Expose AsyncResp shared_ptr when handling response

For Redfish Aggregation, we need a common point to check the D-Bus
for satellite configs. If they are available then we perform the
aggregation op

Expose AsyncResp shared_ptr when handling response

For Redfish Aggregation, we need a common point to check the D-Bus
for satellite configs. If they are available then we perform the
aggregation operations. The functions in query.hpp are used by all
endpoints making them the logical location. The aggregation code
requires a shared_ptr to the AsyncResp so these functions need to be
able to supply that.

This patch is broken out of a future patch for routing Redfish
Aggregation requests
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/53310

The follow commands can be used to perform most of the replacements:
find . -type f | xargs sed -i 's/setUpRedfishRoute(app, req, asyncResp->res/setUpRedfishRoute(app, req, asyncResp/g'
find . -type f | xargs sed -i 's/setUpRedfishRouteWithDelegation(app, req, asyncResp->res/setUpRedfishRouteWithDelegation(app, req, asyncResp/g'

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

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

Fake out allow header in UpdateService

In an attempt to smooth out the transition on changing the URI for
/redfish/v1/UpdateService:HttpPushUri, this patch modifies the allow
header on the /redfish/

Fake out allow header in UpdateService

In an attempt to smooth out the transition on changing the URI for
/redfish/v1/UpdateService:HttpPushUri, this patch modifies the allow
header on the /redfish/v1/UpdateService to return the "wrong" value, and
remove POST from the list of allowed verbs. While this is technically
incorrect, this field is new, so there can't be any users relying on it,
and if they were to use it, they would ideally code to HttpPushUri as
the spec would suggest.

Tested:
Redfish-service-validator passes.

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

show more ...

Change UpdateService POST URI

As d01e32c3786f2fbbb70c9724a87cf979b4a06232 found, the Redfish
specification doesn't allow a direct POST handler on UpdateService.
Ideally clients would be following th

Change UpdateService POST URI

As d01e32c3786f2fbbb70c9724a87cf979b4a06232 found, the Redfish
specification doesn't allow a direct POST handler on UpdateService.
Ideally clients would be following the specification, and relying on
the HttpPushUri as the spec requires, so we could simply make this
change. Unfortunately, a quick polling of the community shows that a
significant number of instances, including the Redfish cheat sheet, and
the robot tests, have hardcoded the non-spec behavior. This commit is
present to give a trap door to allow easier porting of this behavior to
the specification.

The old uri is left, and now returns a WARNING http field, indicating
that the uri is deprecated, in case clients have ignored the Redfish
specification.

Tested:
Ran firmware update instructions from
https://gerrit.openbmc-project.xyz/c/openbmc/docs/+/53664

Test gave the same result as previously.

/redfish/v1/UpdateService returns an HttpPushUri that matches the above.

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

show more ...

Move update service post to free method

Refactor the update service post method in a similar way to how we've
done elsewhere. This is done to enable a refactor later.

Signed-off-by: Ed Tanous <edt

Move update service post to free method

Refactor the update service post method in a similar way to how we've
done elsewhere. This is done to enable a refactor later.

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

show more ...

Remove brace initialization of json objects

Brace initialization of json objects, while quite interesting from an
academic sense, are very difficult for people to grok, and lead to
inconsistencies.

Remove brace initialization of json objects

Brace initialization of json objects, while quite interesting from an
academic sense, are very difficult for people to grok, and lead to
inconsistencies. This patchset aims to remove a majority of them in
lieu of operator[]. Interestingly, this saves about 1% of the binary
size of bmcweb.

This also has an added benefit that as a design pattern, we're never
constructing a new object, then moving it into place, we're always
adding to the existing object, which in the future _could_ make things
like OEM schemas or properties easier, as there's no case where we're
completely replacing the response object.

Tested:
Ran redfish service validator. No new failures.

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

show more ...

Add setUpRedfishRoute to all nodes in redfish

For better or worse, the series ahead of this is making use of
setUpRedfishRoute to do the common "redfish specified" things that need
to be done for a

Add setUpRedfishRoute to all nodes in redfish

For better or worse, the series ahead of this is making use of
setUpRedfishRoute to do the common "redfish specified" things that need
to be done for a connection, like header checking, filtering, and other
things. In the current model, where BMCWEB_ROUTE is a common function
for all HTTP routes, this means we need to propagate this injection call
into the whole tree ahead of the requests being handled.

In a perfect world, we would invent something like a REDFISH_ROUTE
macro, but because macros are discouraged, the routes take a variadic
template of parameters, and each call to the route has a .privileges()
call in the middle, there's no good way to effect this change in a less
costly manner. This was messaged both in the prior reviews, and on
discord sourcing improvements on this pattern, to which none arose.

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

show more ...

Consitently use dbus::utility types

This saves about 4k on the binary size

Tested: Redfish service validator passes.

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

Consitently use dbus::utility types

This saves about 4k on the binary size

Tested: Redfish service validator passes.

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

show more ...