| #
9bc55699
|
| 14-Jun-2022 |
Michal Orzel <michalx.orzel@intel.com> |
Fix for segfault in VirtualMedia.InsertMedia
During Virtual Media legacy mount, on URI validation, at some point boost::urls::result<boost::urls::url_view> object is dereferenced, even when it is in
Fix for segfault in VirtualMedia.InsertMedia
During Virtual Media legacy mount, on URI validation, at some point boost::urls::result<boost::urls::url_view> object is dereferenced, even when it is invalid (this can happen after providing wrong external server in mount configuration). That might eventually cause segmentation fault and bmcweb service to crash. In this fix the problematic dereference is substituted with empty boost::urls::url_view object instead.
Tested: Manual verification in browser; issue doesn't reproduce anymore.
Change-Id: I2ce8891dcea083bae7be65d37574ac1d56905c77 Signed-off-by: Michal Orzel <michalx.orzel@intel.com> Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
| #
3ba00073
|
| 06-Jun-2022 |
Carson Labrado <clabrado@google.com> |
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 ...
|
| #
002d39b4
|
| 31-May-2022 |
Ed Tanous <edtanous@google.com> |
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 ...
|
| #
45ca1b86
|
| 25-Mar-2022 |
Ed Tanous <edtanous@google.com> |
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 ...
|
| #
b9d36b47
|
| 26-Feb-2022 |
Ed Tanous <edtanous@google.com> |
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 ...
|
| #
ace85d60
|
| 26-Oct-2021 |
Ed Tanous <edtanous@google.com> |
Add url type safety to message registry
There are a number of places where we use message registry messages incorrectly. This patchset attempts to fix them, and invoke some type safety when they're
Add url type safety to message registry
There are a number of places where we use message registry messages incorrectly. This patchset attempts to fix them, and invoke some type safety when they're used such that they're more obvious to use.
Namely, it changes a number of the message registry methods to accept a boost::urls::url_view for its argument instead of a const std::string&. This forces the calling code to correctly encode a URL to use the method, which should make it obvious that it's not for an ID, a property name, or anything else. In the course of doing this, several places were found to be using the first argument incorrectly.
Tested: curl --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/Chassis/foobar
Returns: { "error": { "@Message.ExtendedInfo": [ { "@odata.type": "#Message.v1_1_1.Message", "Message": "The requested resource of type #Chassis.v1_16_0.Chassis named foobar was not found.", "MessageArgs": [ "#Chassis.v1_16_0.Chassis", "foobar" ], "MessageId": "Base.1.8.1.ResourceNotFound", "MessageSeverity": "Critical", "Resolution": "Provide a valid resource identifier and resubmit the request." } ], "code": "Base.1.8.1.ResourceNotFound", "message": "The requested resource of type #Chassis.v1_16_0.Chassis named foobar was not found." }
Identically to previously.
Also tested with IDs that contained % encoded characters, like foobar%10, which gave the same result.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Icbb3bce5d190a260610087c9ef35e7becc5a50c7
show more ...
|
| #
15ed6780
|
| 14-Dec-2021 |
Willy Tu <wltu@google.com> |
json_utils: Add support jsonRead Patch/Action
Added support for readJson for Patch and Action. The only difference is that Patch does not allow empty json input while Action does. Action with empty
json_utils: Add support jsonRead Patch/Action
Added support for readJson for Patch and Action. The only difference is that Patch does not allow empty json input while Action does. Action with empty input will use the default value based on the implementation and return 200 OK response code.
readJsonPatch will replace the existing readJson and be used for path requests. It will not allow empty json input and all requested keys are required in the json input.
readJsonAction will be used for Action requests where it is possible for all of the properties to be optional and allow empty request. The optional properties are determined by the requested values type.
All current Action readJson are replaced with readJsonAction. It does not change the existing behavior since it needs `std::optional`. This will have to be updated later as we define the default behavior.
Tested: Added unit tests and readJsonAction allows empty empty json object.
No Change to Redfish Tree.
Change-Id: Ia5e1f81695c528a20f1dc985aee19c920d8adaea Signed-off-by: Willy Tu <wltu@google.com>
show more ...
|
| #
e662eae8
|
| 25-Jan-2022 |
Ed Tanous <edtanous@google.com> |
Enable readability-implicit-bool-conversion checks
These checks ensure that we're not implicitly converting ints or pointers into bools, which makes the code easier to read.
Tested: Ran series thro
Enable readability-implicit-bool-conversion checks
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
show more ...
|
| #
e05aec50
|
| 25-Jan-2022 |
Ed Tanous <edtanous@google.com> |
Add readability-redundant-* checks
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@go
Add readability-redundant-* checks
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
show more ...
|
| #
50b8a43a
|
| 03-Feb-2022 |
Ed Tanous <edtanous@google.com> |
Remove NEW_BOOST_URL macro
Now that the subtree update is done, this define is no longer needed.
Tested: Code compiles. Noop.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Idc5d7ef69c
Remove NEW_BOOST_URL macro
Now that the subtree update is done, this define is no longer needed.
Tested: Code compiles. Noop.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Idc5d7ef69c009982a2476fadc1d95e3280bfff48
show more ...
|
| #
67df073b
|
| 26-Oct-2021 |
Ed Tanous <edtanous@google.com> |
Next round of boost-uri updates
Boost url has changed some APIs again. This commit updates our URIs to handle it. As part of this work, it also removes some of the debug prints that were put in ea
Next round of boost-uri updates
Boost url has changed some APIs again. This commit updates our URIs to handle it. As part of this work, it also removes some of the debug prints that were put in early on. These aren't really needed these days.
This commit invents a temporary #define of NEW_BOOST_URL, so we can get through the subtree update without a hard dependency on this specific version of bmcweb. Ideally boost-url would have some version field, but unfortunately, it is thusfar unversioned, as the long term intent of the author is to be included in boost, and would be versioned there.
All the code within the else of the NEW_BOOST_URL flag will be removed once the subtree update is landed.
Tested: Added CXXFLAGS:append = " -DNEW_BOOST_URL" to the recipe and checked out on top of the subtree update, and build succeeded.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie2064e45efbc4331bdc5a5ddf44d877cde5e13cb
show more ...
|
| #
26f6976f
|
| 25-Jan-2022 |
Ed Tanous <edtanous@google.com> |
Enable readability-container-size-empty tests
This one is a little trivial, but it does help in readability.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I5366d4eec8af2f781b3bad804131a
Enable readability-container-size-empty tests
This one is a little trivial, but it does help in readability.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I5366d4eec8af2f781b3bad804131ae2eb806e3aa
show more ...
|
| #
4ecc618f
|
| 07-Jan-2022 |
Ed Tanous <edtanous@google.com> |
Enable const_cast checks
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 T
Enable const_cast checks
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
show more ...
|
| #
ecd6a3a2
|
| 07-Jan-2022 |
Ed Tanous <edtanous@google.com> |
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 ...
|
| #
914e2d5d
|
| 07-Jan-2022 |
Ed Tanous <edtanous@google.com> |
Enforce const correctness
For all async calls, we should be consistently capturing non trivial objects by const reference. This corrects bmcweb to be consistent and capture errors by const value, a
Enforce const correctness
For all async calls, we should be consistently capturing non trivial objects by const reference. This corrects bmcweb to be consistent and capture errors by const value, and objects by const reference.
Tested: Code compiles. Trivial changes.
This saves about 300 bytes on our compressed binary size.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib3e0b6edef9803a1c480701556949488406305d4
show more ...
|
| #
711ac7a9
|
| 20-Dec-2021 |
Ed Tanous <edtanous@google.com> |
Consistently use ManagedObjectType
Some subsystems seem to have invented their own typedefs for this stuff, move to using the one typedef in dbus::utility so we're consistent, and we reduce our temp
Consistently use ManagedObjectType
Some subsystems seem to have invented their own typedefs for this stuff, move to using the one typedef in dbus::utility so we're consistent, and we reduce our templates.
Tested: code compiles
This saves a negligible amount (104 bytes compressed) on our binary size.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I952ea1f960aa703808d0ac80f35dc24cdd8d5027
show more ...
|
| #
168e20c1
|
| 13-Dec-2021 |
Ed Tanous <edtanous@google.com> |
Move to common variant
This saves approximately 34kB in the compressed binary size of bmcweb due to reduced template instantiations. This amounts to a 2.5% reduction in the overall size.
Note, the
Move to common variant
This saves approximately 34kB in the compressed binary size of bmcweb due to reduced template instantiations. This amounts to a 2.5% reduction in the overall size.
Note, there were a few places where we broke const-correctness in the form of pulling a non-const reference out of a const variant. This new variant now requires const correctness, so some consts are added where required.
Tested: Code compiles.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I6a60c8881c1268627eedb4ffddf16689dc5f6ed2
show more ...
|
| #
98be3e39
|
| 16-Sep-2021 |
Ed Tanous <edtanous@google.com> |
Remove copies from log services and virtual media
Both of these entries make complete copies of the Request object. Following the pattern in the prior commit, move these to more modern patterns.
Fo
Remove copies from log services and virtual media
Both of these entries make complete copies of the Request object. Following the pattern in the prior commit, move these to more modern patterns.
For log service, this simply means constructing a payload object earlier.
For virtual media, it means doing the readJson call and parameter validation much earlier, which generally is the pattern we should strive for, validating and unpacking the structs in the first scope, then dealing with them as structures. To do this, this commit also creates a secondary struct to store the param data in to make the lambdas cleaner.
Tested: From Ashmitha
POST https://${bmc}/redfish/v1/Managers/bmc/LogServices/Dump/Actions/LogService.CollectDiagnosticData -d '{"DiagnosticDataType":"Manager"}' {
"@odata.id": "/redfish/v1/TaskService/Tasks/0", "@odata.type": "#Task.v1_4_3.Task", "Id": "0", "TaskState": "Running", "TaskStatus": "OK" } ----------------------------------------------------------------- On task completion: GET https://${bmc}/redfish/v1/TaskService/Tasks/0 {
"@odata.id": "/redfish/v1/TaskService/Tasks/0", "@odata.type": "#Task.v1_4_3.Task", "EndTime": "2021-12-03T13:40:58+00:00", "Id": "0", "Messages": [ { "@odata.type": "#Message.v1_0_0.Message", "Message": "The task with id 0 has started.", "MessageArgs": [ "0" ], "MessageId": "TaskEvent.1.0.1.TaskStarted", "Resolution": "None.", "Severity": "OK" }, { "@odata.type": "#Message.v1_1_1.Message", "Message": "Successfully Completed Request", "MessageArgs": [], "MessageId": "Base.1.8.1.Success", "MessageSeverity": "OK", "Resolution": "None" } ], "Name": "Task 0", "Payload": { "HttpHeaders": [ "Host: 9.3.29.238", "User-Agent: curl/7.71.1", "Accept: */*", "Content-Length: 32", "Location: /redfish/v1/Managers/bmc/LogServices/Dump/Entries/32" ], "HttpOperation": "POST", "JsonBody": "{\n \"DiagnosticDataType\": \"Manager\"\n}", "TargetUri": "/redfish/v1/Managers/bmc/LogServices/Dump/Actions/LogService.CollectDiagnosticData" }, "PercentComplete": 0, "StartTime": "2021-12-03T13:38:20+00:00", "TaskMonitor": "/redfish/v1/TaskService/Tasks/0/Monitor", "TaskState": "Completed", "TaskStatus": "OK" }
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I178a3a7a7b27dfd34ec50a06398ac243a9d4ab67
show more ...
|
| #
0fda0f12
|
| 15-Nov-2021 |
George Liu <liuxiwei@inspur.com> |
Update clang-format
refer: https://github.com/openbmc/docs/blob/master/style/cpp/.clang-format `Don't break long string literals`
Tested: built bmcweb successfully and RedfishValidator Passed.
Sig
Update clang-format
refer: https://github.com/openbmc/docs/blob/master/style/cpp/.clang-format `Don't break long string literals`
Tested: built bmcweb successfully and RedfishValidator Passed.
Signed-off-by: George Liu <liuxiwei@inspur.com> Change-Id: Ib58f7c942fd3838592e043c57e0b6ffcdc3d963b
show more ...
|
| #
d32c4fa9
|
| 14-Sep-2021 |
Ed Tanous <edtanous@google.com> |
Boost uri update
Update to the latest version of boost::uri
The newest version of boost uri makes some breaking changes that we need to account for. At the same time, we take the opportunity to mo
Boost uri update
Update to the latest version of boost::uri
The newest version of boost uri makes some breaking changes that we need to account for. At the same time, we take the opportunity to move to the error code based parse methods that don't rely on exceptions.
The biggest changes are: The standalone build is no longer present. A discussion with the boost::url maintainers shows that our best option is to do a simple copy of the headers, and compile boost/url/src.hpp in a separate file. This is intended to allow people to pull the library in "standalone" and not have to rely on the build machinery in boost-url, which we don't really need. Interestingly, this file doesn't have a newline at the end, which clang correctly flags. OpenBMC doesn't really need that warning, as we rely on clang-format to do that, so we add -Wno-newline-eof clang to get the code to compile there.
All url parsers are moved to the parse_uri, or parse_relative_uri equivalents. This slightly tightens the requirements around what URLs are accepted, but in no ways that should break anything. (Ie, "/redfish/v1" is no longer accepted for a virtual media endpoint.
boost::urls::url_view::params_type has been renamed to query_params_type, and the relevant methods have been updated.
Because of the missing standalone mode, we now need to use boost::string_view which doesn't implicitly construct from std::string_view. Some discussion on the boost list shows that this is coming soon, so that cruft can eventually be cleaned up, but for now we need the construction.
Tested: Loaded in qemu, and ran some URLs (/redfish/v1 and /redfish/v1/Chassis) to ensure that the url handler functions as intended.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I5843776d4ec01b4d92af2ee3a9cf1ebb1d920ae7
show more ...
|
| #
ed398213
|
| 09-Jun-2021 |
Ed Tanous <edtanous@google.com> |
Automate PrivilegeRegistry to code
This commit attempts to automate the creation of our privileges structures from the redfish privilege registry. It accomplishes this by updating parse_registries.
Automate PrivilegeRegistry to code
This commit attempts to automate the creation of our privileges structures from the redfish privilege registry. It accomplishes this by updating parse_registries.py to also pull down the privilege registry from DMTF. The script then generates privilege_registry.hpp, which include const defines for all the privilege registry entries in the same format that the Privileges struct accepts. This allows new clients to simply reference the variable to these privilege structures, instead of having to manually (ie error pronely) put the privileges in themselves.
This commit updates all the routes.
For the moment, override and OEM schemas are not considered. Today we don't have any OEM-specific Redfish routes, so the existing ones inherit their parents schema. Overrides have other issues, and are already incorrect as Redfish defines them.
Binary size remains unchanged after this patchset.
Tested: Ran redfish service validator
Ran test case from f9a6708c4c6490257e2eb6a8c04458f500902476 to ensure that the new privileges constructor didn't cause us to regress the brace construction initializer.
Checked binary size with: gzip -c $BBPATH/tmp/work/s7106-openbmc-linux-gnueabi/obmc-phosphor-image/1.0-r0/rootfs/usr/bin/bmcweb | wc -c 1244048
(tested on previous patchset)
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ideede3d5b39d50bffe7fe78a0848bdbc22ac387f
show more ...
|
| #
432a890c
|
| 14-Jun-2021 |
Ed Tanous <edtanous@google.com> |
Remove ambiguous privileges constructor
There are a number of endpoints that assume that a given routes privileges are governed by a single set of privileges, instead of multiple sets ORed together.
Remove ambiguous privileges constructor
There are a number of endpoints that assume that a given routes privileges are governed by a single set of privileges, instead of multiple sets ORed together. To handle this, there were two overloads of the privileges() method, one that took a vector of Privileges, and one that took an initializer_list of const char*. Unfortunately, this leads some code in AccountService to pick the wrong overload when it's called like this .privileges( {{"ConfigureUsers"}, {"ConfigureManager"}, {"ConfigureSelf"}})
This is supposed to be "User must have ConfigureUsers, or ConfigureManager, or ConfigureSelf". Currently, because it selects the wrong overload, it computes to "User must have ConfigureUsers AND ConfigureManager AND ConfigureSelf.
The double braces are supposed to cause this to form a vector of Privileges, but it appears that the initializer list gets consumed, and the single invocation of initializer list is called. Interestingly, trying to put in a privileges overload of intializer_list<initializer_list<const char*>> causes the compilation to fail with an ambiguous call error, which is what I would've expected to see previously in this case, but alas, I'm only a novice when it comes to how the C++ standard works in these edge cases. This is likely due in part to the fact that they were templates of an unused template param (seemingly copied from the previous method) and SFINAE rules around templates.
This commit functionally removes one of the privileges overloads, and adds a second set of braces to every privileges call that previously had a single set of braces. Previous code will not compile now, which is IMO a good thing.
This likely popped up in the Node class removal, because the Node class explicitly constructs a vector of Privilege objects, ensuing it can hit the right overload
Tested: Ran Redfish service validator
Tested the specific use case outlined on discord with: Creating a new user with operator privilege: ``` redfishtool -S Always -u root -p 0penBmc -vvvvvvvvv -r 192.168.7.2 AccountService adduser foo mysuperPass1 Operator ```
Then attempting to list accounts: ``` curl -vvvv --insecure --user foo:mysuperPass1 https://192.168.7.2/redfish/v1/AccountService/Accounts/foo ```
Which succeeded and returned the account in question.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I83e62b70e97f56dc57d43b9081f333a02fe85495
show more ...
|
| #
22db1728
|
| 09-Jun-2021 |
Ed Tanous <edtanous@google.com> |
Remove Node class from NBD proxy
This was broken as part of the recent Node removal. Although this code is currently dead, it would be good if at least was able to be built, so if someone was incli
Remove Node class from NBD proxy
This was broken as part of the recent Node removal. Although this code is currently dead, it would be good if at least was able to be built, so if someone was inclined to upstream the backend for it, they could without too much trouble.
Because of this move in code, clang-tidy can now see this code and found a number of failures, so this is combined with a couple of relatively trivial clang-tidy changes that are low impact and mechanical in nature.
Tested: Commented in the option in meson_options.txt, and built. Code builds.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I5ff52b773fc4fc9d44a916c4e6bf6e60295c3aaa
show more ...
|
| #
b282a438
|
| 03-Jun-2021 |
Ed Tanous <edtanous@google.com> |
Remove the Node class
Fixes #181
Lots of specific details around why the node class have been removed are in the previous patchsets. This commit actually does the deed and makes it go away entirel
Remove the Node class
Fixes #181
Lots of specific details around why the node class have been removed are in the previous patchsets. This commit actually does the deed and makes it go away entirely.
Now that this is finally done, we can compare binary size. Surprisingly enough, this series saves a full 72KB of compressed binary size, which amounts to about 6.4% of the total code size.
Before: 1197632 bytes After: 1124688 bytes
This IMO makes it worth it, considering we've significantly reduced the amount of code at the same time.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I3c8688715f933b381cad0be75a079ccfd72c3130
show more ...
|
| #
7e860f15
|
| 08-Apr-2021 |
John Edward Broadbent <jebr@google.com> |
Remove Redfish Node class
Reduces the total number of lines and will allow for easier testing of the redfish responses.
A main purpose of the node class was to set app.routeDynamic(). However now a
Remove Redfish Node class
Reduces the total number of lines and will allow for easier testing of the redfish responses.
A main purpose of the node class was to set app.routeDynamic(). However now app.routeDynamic can handle the complexity that was once in critical to node. The macro app.routeDynamic() provides a shorter cleaner interface to the unerlying app.routeDyanic call. The old pattern set permissions for 6 interfaces (get, head, patch, put, delete_, and post) even if only one interface is created. That pattern creates unneeded code that can be safely removed with no effect. Unit test for the responses would have to mock the node the class in order to fully test responses.
see https://github.com/openbmc/bmcweb/issues/181
The following files still need node to be extracted.
virtual_media.hpp account_service.hpp redfish_sessions.hpp ethernet.hpp
The files above use a pattern that is not trivial to address. Often their responses call an async lambda capturing the inherited class. ie (https://github.com/openbmc/bmcweb/blob/ffed87b5ad1797ca966d030e7f979770 28d258fa/redfish-core/lib/account_service.hpp#L1393) At a later point I plan to remove node from the files above.
Tested: I ran the docker unit test with the following command. WORKSPACE=$(pwd) UNIT_TEST_PKG=bmcweb ./openbmc-build-scripts/run-unit-test-docker.sh
I ran the validator and this change did not create any issues. python3 RedfishServiceValidator.py -c config.ini
Signed-off-by: John Edward Broadbent <jebr@google.com> Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I147a0289c52cb4198345b1ad9bfe6fdddf57f3df
show more ...
|